This is an archive of the discontinued LLVM Phabricator instance.

[Clang][test] fix tests when using external assembler
ClosedPublic

Authored by ychen on May 22 2020, 12:38 PM.

Details

Summary

The test assume using integraed-as, so make it explicit.

Diff Detail

Event Timeline

ychen created this revision.May 22 2020, 12:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 22 2020, 12:38 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
ychen planned changes to this revision.May 22 2020, 12:46 PM

This may not be complete.

ychen updated this revision to Diff 265791.May 22 2020, 12:55 PM
  • update
ychen retitled this revision from [Clang] fix limiting -fintegrated-cc1 to only one TU to [Clang][test] fix tests when using external assembler.May 22 2020, 12:55 PM
ychen edited the summary of this revision. (Show Details)

Could you please attach a test to demonstrate the issue?

ychen updated this revision to Diff 265792.May 22 2020, 12:56 PM
  • update
ychen added a comment.EditedMay 22 2020, 12:59 PM

Could you please attach a test to demonstrate the issue?

When a toolchain is not using integrated-as, the test would fail because using external assember would increase job counts. Most toolchains in tree use integrated-as, but we still support external assembler. So this is just making it explicit.

aganea accepted this revision.May 22 2020, 2:31 PM

LGTM.

Are you planning on adding another patch for the change that you've sent initially? Ideally, I'd like to relax a bit that constraint in Driver.cpp, as in D74447, but that requires first to disable -disable-free to ensure we exit cleanly, at least when runing the tests (a bit like the LLD_IN_TEST env var).

This revision is now accepted and ready to land.May 22 2020, 2:31 PM
MaskRay added a subscriber: MaskRay.EditedMay 22 2020, 2:45 PM

Could you please attach a test to demonstrate the issue?

When a toolchain is not using integrated-as, the test would fail because using external assember would increase job counts. Most toolchains in tree use integrated-as, but we still support external assembler. So this is just making it explicit.

This point deserves a comment in the test.

@ychen What is your LLVM_DEFAULT_TARGET_TRIPLE?

Harbormaster completed remote builds in B57676: Diff 265791.
ychen added a comment.May 22 2020, 3:09 PM

Could you please attach a test to demonstrate the issue?

When a toolchain is not using integrated-as, the test would fail because using external assember would increase job counts. Most toolchains in tree use integrated-as, but we still support external assembler. So this is just making it explicit.

This point deserves a comment in the test.

Will do.

@ychen What is your LLVM_DEFAULT_TARGET_TRIPLE?

All I could say is that it is not a in-tree toolchain and it uses external assembler.

ychen added a comment.May 22 2020, 4:46 PM

LGTM.

Are you planning on adding another patch for the change that you've sent initially? Ideally, I'd like to relax a bit that constraint in Driver.cpp, as in D74447, but that requires first to disable -disable-free to ensure we exit cleanly, at least when runing the tests (a bit like the LLD_IN_TEST env var).

Oh, that's not my plan. I think the motivation of D74447 makes great sense.

This revision was automatically updated to reflect the committed changes.