This is an archive of the discontinued LLVM Phabricator instance.

[lld/mac] Give several LTO tests an "lto-" prefix
ClosedPublic

Authored by thakis on Jul 6 2021, 6:30 AM.

Details

Reviewers
gkm
int3
Group Reviewers
Restricted Project
Commits
rG76f734040a54: [lld/mac] Give several LTO tests an "lto-" prefix

Diff Detail

Event Timeline

thakis created this revision.Jul 6 2021, 6:30 AM
Herald added a project: Restricted Project. · View Herald Transcript
thakis requested review of this revision.Jul 6 2021, 6:30 AM
int3 added a subscriber: int3.Jul 6 2021, 8:20 AM

I intentionally omitted the lto- prefix from this (and several other tests) because it seemed redundant -- practically all our .ll tests are LTO tests. I guess that's not strictly true any more though given things like lc-linker-option.ll. There are also some LTO tests that do have the lto- prefix, so we're not consistent about it either. I guess it doesn't really matter which scheme we pick as long as we're consistent. So... if we are going to add said prefix, can we do it for all the other LTO tests as well?

thakis updated this revision to Diff 356761.Jul 6 2021, 9:29 AM
thakis retitled this revision from [lld/mac] Rename internalize.ll test to lto-internalize.ll to [lld/mac] Give several LTO tests an "lto-" prefix.

I intentionally omitted the lto- prefix from this (and several other tests) because it seemed redundant -- practically all our .ll tests are LTO tests. I guess that's not strictly true any more though given things like lc-linker-option.ll. There are also some LTO tests that do have the lto- prefix, so we're not consistent about it either. I guess it doesn't really matter which scheme we pick as long as we're consistent. So... if we are going to add said prefix, can we do it for all the other LTO tests as well?

I had tried that, but apparently did a very poor job. I had looked at rg llvm-as lld/test/MachO, rg save-temps lld/test/MachO but apparently not very closedly.

Now that I look through lld/test/MachO/*.ll I found several else. I did _not_ rename these files since they don't seem primarily LTO tests:

  • lld/test/MachO/bitcode-bundle.ll
  • lld/test/MachO/bitcode-nodatalayout.ll (…should this one be in invalid/?)
  • lld/test/MachO/force-load-swift-libs.ll
  • lld/test/MachO/lc-linker-option.ll
int3 accepted this revision.Jul 6 2021, 10:57 AM

lld/test/MachO/bitcode-nodatalayout.ll (…should this one be in invalid/?)

yeah, putting it under invalid/ would probably make more sense. I think it should be considered an lto- test too, since the error does come from the LTO compilation. LLD-ELF does put it under test/ELF/lto/.

Thanks for the cleanup!

This revision is now accepted and ready to land.Jul 6 2021, 10:57 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 6 2021, 12:24 PM