Page MenuHomePhabricator

Fix ARM build bots failures due to disabled x86_64-apple target
AbandonedPublic

Authored by JamesNagurne on Aug 3 2020, 12:08 PM.

Details

Summary
Revision https://reviews.llvm.org/D69384 added the option "-codegenprepare" to an invocation of
opt, which then requires LLVM to have the expected target available for
code generation.

The ARM build bots do not compile this target as part of their build,
causing this test to abort:

"Trying to construct TargetPassConfig without a target machine.
Scheduling a CodeGen pass without a target triple set?"

Moving this test to the X86 subdirectory ensures that builds without the
X86 backend skip this test.

Diff Detail

Event Timeline

JamesNagurne created this revision.Aug 3 2020, 12:08 PM
JamesNagurne requested review of this revision.Aug 3 2020, 12:08 PM
JamesNagurne set the repository for this revision to rG LLVM Github Monorepo.
JamesNagurne edited the summary of this revision. (Show Details)

I apologize for the constant early changes. I forget how phabricator catches and marks things as being referenced, so I was trying to get it to work.
This is a patch fix for D69384, which has broken ARM build bots. I present this as a band-aid since this feature and its test seems to be rather generic.

dyung added a subscriber: dyung.Aug 4 2020, 1:17 AM
dyung added inline comments.
llvm/test/Transforms/HotColdSplit/X86/coldentrycount.ll
4

Shouldn't this be x86-registered-target? If I build the compiler with an x86-only backend, but not an apple triple as the default triple, I think this test gets marked as unsupported with your change instead of being run.

RKSimon added a subscriber: RKSimon.Aug 4 2020, 1:32 AM

Is there any reason not to just move this file into llvm/test/Transforms/HotColdSplit/X86 where the existing lit filters will handle it?

Either of these options sounds good to me.

@RKSimon At this point I'd tend to agree that this test belongs in the X86 subdirectory. I was hoping the original developer would come in and find a way to make this test generic once again, since doing so reduces test coverage in general.

JamesNagurne edited the summary of this revision. (Show Details)

Moved test to X86 subdirectory

JamesNagurne abandoned this revision.Aug 4 2020, 8:57 AM

Abandoning in lieu of fix in D85215

rjf added a subscriber: rjf.Aug 4 2020, 9:02 AM

@JamesNagurne We proposed a fix in https://reviews.llvm.org/D85215, instead of moving it into X86/, we made the test cases architecture-agnostic.

This looks like a more reasonable fix for now.

Reverted my original diff; D85229