This is an archive of the discontinued LLVM Phabricator instance.

Add lit test for Win
Needs ReviewPublic

Authored by yinyangsx on Jun 2 2023, 2:07 AM.

Event Timeline

yinyangsx created this revision.Jun 2 2023, 2:07 AM
Herald added a project: Restricted Project. · View Herald Transcript
yinyangsx requested review of this revision.Jun 2 2023, 2:07 AM
aaron.ballman added subscribers: Restricted Project, aaron.ballman.

Thank you for this!

buildbot/osuosl/master/config/builders.py
864

Based on internal discussions, should we -DCMAKE_BUILD_TYPE=Debug so that we enable the checked STL functionality and get good stack traces when there are failures. Or are we going to stand up a different bot for that use case?

yinyangsx updated this revision to Diff 529122.Jun 6 2023, 7:05 PM
  • Add -DCMAKE_BUILD_TYPE=Debug option
yinyangsx updated this revision to Diff 529124.Jun 6 2023, 7:08 PM
  • Add -DCMAKE_BUILD_TYPE=Debug option
yinyangsx updated this revision to Diff 529127.Jun 6 2023, 7:16 PM
  • Add -DCMAKE_BUILD_TYPE=Debug option
gkistanova requested changes to this revision.Jun 6 2023, 7:30 PM

Hi Yin,

Could you remove from this patch anything and everything unrelated to the change you are proposing, please?
Did you forget to rebase onto the HEAD?

This revision now requires changes to proceed.Jun 6 2023, 7:30 PM

Hi, sorry for that, I am trying to move them but I not familiar with arc, so I need more time

No worries.
As another option, if you have a clean diff, you could use the WebUI to upload that. There is "Update Diff" link on the right top corner of this screen. Click that and follow the sequence of the pages. Maybe it would be easier?

yinyangsx updated this revision to Diff 529136.Jun 6 2023, 7:46 PM

Thanks for updating the patch!

It looks good with a couple of notes.

buildbot/osuosl/master/config/builders.py
905

-DLLVM_ENABLE_ASSERTIONS defaults to ON for Debug builds.

910

You do not need extra quotation marks around X86 here.

Thank you! I have resolved the problem.

This revision is now accepted and ready to land.Jun 6 2023, 8:03 PM

hi Galina, Thank you a lot for your help, could you please land this patch?

The last diff look like incorrect, I've made an update.

aaron.ballman accepted this revision.Jun 7 2023, 5:47 AM

LGTM, thank you!!!

This revision was automatically updated to reflect the committed changes.
gkistanova reopened this revision.Jun 8 2023, 10:22 AM

Apparently ClangBuilder does not support vs="autodetect" along with the test suite.

The problem is in these lines:

test_suite_env = copy.deepcopy(env)
test_suite_env['CC'] = cc
test_suite_env['CXX'] = cxx

In case of "autodetect" env is a build property, not a dictionary, and it does not support explicit assignments to CC and CXX.

I'll see if I can get some time to look into this issue over weekend or earlier next week. Please feel free to propose a patch if somebody else has some spare time to fix this issue, .

In the mean time, @aaron.ballman, could you revert this patch, please?

This revision is now accepted and ready to land.Jun 8 2023, 10:22 AM

Apparently ClangBuilder does not support vs="autodetect" along with the test suite.

The problem is in these lines:

test_suite_env = copy.deepcopy(env)
test_suite_env['CC'] = cc
test_suite_env['CXX'] = cxx

In case of "autodetect" env is a build property, not a dictionary, and it does not support explicit assignments to CC and CXX.

I'll see if I can get some time to look into this issue over weekend or earlier next week. Please feel free to propose a patch if somebody else has some spare time to fix this issue, .

In the mean time, @aaron.ballman, could you revert this patch, please?

I've reverted in 14f85d830c9ba8eaabcc1f6d35e90c6fc995a173, thank you for catching this!

  • Add an MSVC debug x64 builder
gkistanova requested changes to this revision.Jun 19 2023, 11:07 AM

Could you remove any and all unrelated changes from this patch, please?
What was wrong with the original approved patch?

This revision now requires changes to proceed.Jun 19 2023, 11:07 AM

Could you remove any and all unrelated changes from this patch, please?
What was wrong with the original approved patch?

I created this patch by mistake. Could you please tell me how to close it?

This differential revision is referenced from the commit and is a part of the git history. So, you need to restore the diff to the original form which has been approved and committed.

There are multiple possible ways of doing so, the easiest, I think, is just download the raw diff from https://reviews.llvm.org/file/data/6esxbhtveros4tnrzliu/PHID-FILE-ygiily67vdz436p5gmmb/file, then click "Update Diff" here (top right part of the screen), and upload that saved diff.

Please let me know if you need a help with this.

Once the diff is updated to the correct one, leave this review as is without changing anything else.

yinyangsx updated this revision to Diff 536936.Jul 3 2023, 7:37 PM
  • Update LLDB Arm/AArch64 email notifier list
  • Add an MSVC debug x64 builder

This looks to have picked up a bunch of unrelated changes again.

I don't know why many changes has been submitted when I run "arc diff" and I can't delete these changes. So I created a new one to replace it, https://reviews.llvm.org/D154406

zorg/jenkins/jobs/jobs/lldb-cmake-matrix