The tests only specify -march, so when the tests are run on AIX the target OS defaults to AIX, which causes the tests to misbehave.
This patch constrains the tests by specifying -mtriple instead of -march.
Paths
| Differential D110186
Fix tests defaulting to incorrect triples on AIX ClosedPublic Authored by Jake-Egan on Sep 21 2021, 10:23 AM.
Details Summary The tests only specify -march, so when the tests are run on AIX the target OS defaults to AIX, which causes the tests to misbehave. This patch constrains the tests by specifying -mtriple instead of -march.
Diff Detail
Unit TestsFailed Event TimelineHerald added subscribers: atanasyan, jrtc27, fedor.sergeev and 3 others. · View Herald TranscriptSep 21 2021, 10:23 AM Comment Actions Just a general question, how did you determine the OS and environment type used in the triple? Comment Actions
From other tests that set the triple. Comment Actions Should they be -mtriple=foo-unknown-linux or just -mtriple=foo, i.e. foo-unknown-elf? Given they've worked fine on Linux, FreeBSD and macOS for years I assume they don't care about the OS part of the triple (except for whatever AIX is doing that causes them to break...) and so should just use a generic ELF triple? Comment Actions I don't think this is a correct way to fix all these for AIX. Since we don't really support those arch on AIX, we should update getMultiarchTriple to resolve the arch to correct triple, instead of trying to update all existing tests. This revision now requires changes to proceed.Sep 21 2021, 1:39 PM Comment Actions
Can you confirm? I don't think so. See llvm/lib/Support/Triple.cpp:getDefaultFormat which is used to normalize a triple when the format is not explicitly specified. ELF is the default binary format for everything except wasm. Comment Actions
-march is defined to be "the native triple with the CPU replaced with the argument" and has caused problems in the past. It's not the same as -mtriple where there are defaults that don't depend on the native triple. Comment Actions Hmm, for context the error is related to the relocation model: ./bin/llc -filetype=obj -march=sparcv9 --relocation-model=static < foo.ll llc: error: '<stdin>': invalid relocation model, AIX only supports PIC I guess if we wanted to side step this issue maybe we could tie that error to the object file format (i.e. XCOFF) instead and presumably we wouldn't see this error since the default is ELF? Comment Actions
Looks like -march may be a bit misleading due to llvm/lib/Support/Triple.cpp:getDefaultFormat returning ELF for many machines. For example, on arm64 macOS, TheTarget->createTargetMachine takes a sparcv9-apple-darwin20.3.0 triple while the binary format is ELF since Darwin doesn't support sparcv9. I think switching to -mtriple= makes sense to me. Either -mtriple=sparc9 or -mtriple=sparc9-unknown-elf works for me, perhaps the former for brevity. Comment Actions
So, the _triple_ is always exactly what I said. But if that's a nonsensical triple, such as sparcv9-apple-darwin20.3.0, then yes things can get weird and it may end up falling back on ELF until suddenly that case gets implemented properly. Comment Actions I am OK with this if others accept it. But as I mentioned, this is not a good way to fix it -- we will have to deal with any new testcases using -march in the future. This revision is now accepted and ready to land.Sep 27 2021, 6:52 AM Comment Actions
No, tests that use -march= are broken, they will by definition have host-dependent behaviour. Comment Actions
Yes, that is why I am asking to update the host (*AIX*) default behaviors for those arch we don't support -- we don't have to default to XCOFF for those arch on AIX. Comment Actions
My point is this is the correct way to fix these broken tests; you were saying this was not a good way to fix it. Comment Actions
:), you are right. I agree that this is the correct way. This revision was landed with ongoing or failed builds.Sep 27 2021, 8:31 AM Closed by commit rG56049b71294c: Fix tests defaulting to incorrect triples on AIX (authored by Jake-Egan). · Explain Why This revision was automatically updated to reflect the committed changes. Comment Actions Due to its weird semantics, -march is probably only suitable for quick local testing but not for the testsuite.
Revision Contents
Diff 373978 llvm/test/CodeGen/Mips/2008-07-15-InternalConstant.ll
llvm/test/CodeGen/Mips/2008-07-15-SmallSection.ll
llvm/test/CodeGen/Mips/2009-11-16-CstPoolLoad.ll
llvm/test/CodeGen/Mips/2010-07-20-Switch.ll
llvm/test/CodeGen/Mips/Fast-ISel/bswap1.ll
llvm/test/CodeGen/Mips/Fast-ISel/callabi.ll
llvm/test/CodeGen/Mips/Fast-ISel/memtest1.ll
llvm/test/CodeGen/Mips/addi.ll
llvm/test/CodeGen/Mips/align16.ll
llvm/test/CodeGen/Mips/blockaddr.ll
llvm/test/CodeGen/Mips/br-jmp.ll
llvm/test/CodeGen/Mips/brdelayslot.ll
llvm/test/CodeGen/Mips/brind-tailcall.ll
llvm/test/CodeGen/Mips/cconv/arguments-float.ll
llvm/test/CodeGen/Mips/cconv/arguments-fp128.ll
llvm/test/CodeGen/Mips/cconv/arguments-hard-float-varargs.ll
llvm/test/CodeGen/Mips/cconv/arguments-hard-float.ll
llvm/test/CodeGen/Mips/cconv/arguments-hard-fp128.ll
llvm/test/CodeGen/Mips/cconv/arguments-varargs.ll
llvm/test/CodeGen/Mips/cconv/arguments.ll
llvm/test/CodeGen/Mips/cconv/return-hard-fp128.ll
llvm/test/CodeGen/Mips/compactbranches/compact-branches.ll
llvm/test/CodeGen/Mips/copy-fp64.ll
llvm/test/CodeGen/Mips/delay-slot-fill-forward.ll
llvm/test/CodeGen/Mips/fp16static.ll
llvm/test/CodeGen/Mips/fpneeded.ll
llvm/test/CodeGen/Mips/global-address.ll
llvm/test/CodeGen/Mips/gpopt-explict-section.ll
llvm/test/CodeGen/Mips/indirectcall.ll
llvm/test/CodeGen/Mips/interrupt-attr-64-error.ll
llvm/test/CodeGen/Mips/interrupt-attr-args-error.ll
llvm/test/CodeGen/Mips/interrupt-attr-error.ll
llvm/test/CodeGen/Mips/interrupt-attr.ll
llvm/test/CodeGen/Mips/jtstat.ll
llvm/test/CodeGen/Mips/micromips-delay-slot-jr.ll
llvm/test/CodeGen/Mips/micromips-delay-slot.ll
llvm/test/CodeGen/Mips/micromips-jal.ll
llvm/test/CodeGen/Mips/micromips-rdhwr-directives.ll
llvm/test/CodeGen/Mips/mips16_32_10.ll
llvm/test/CodeGen/Mips/mips16_32_3.ll
llvm/test/CodeGen/Mips/mips16_32_4.ll
llvm/test/CodeGen/Mips/mips16_32_5.ll
llvm/test/CodeGen/Mips/mips16_32_6.ll
llvm/test/CodeGen/Mips/mips16_32_7.ll
llvm/test/CodeGen/Mips/mips16_32_8.ll
llvm/test/CodeGen/Mips/mips16_32_9.ll
llvm/test/CodeGen/Mips/mirparser/target-flags-static-tls.mir
llvm/test/CodeGen/Mips/mno-ldc1-sdc1.ll
llvm/test/CodeGen/Mips/named-register-n32.ll
llvm/test/CodeGen/Mips/named-register-n64.ll
llvm/test/CodeGen/Mips/named-register-o32.ll
llvm/test/CodeGen/Mips/no-frame-pointer-elim.ll
llvm/test/CodeGen/Mips/rdhwr-directives.ll
llvm/test/CodeGen/Mips/stacksize.ll
llvm/test/CodeGen/Mips/tailcall/tailcall-wrong-isa.ll
llvm/test/CodeGen/Mips/tailcall/tailcall.ll
llvm/test/CodeGen/NVPTX/globals_lowering.ll
llvm/test/CodeGen/SPARC/blockaddr.ll
llvm/test/CodeGen/SPARC/constpool.ll
llvm/test/CodeGen/SPARC/exception.ll
llvm/test/CodeGen/SPARC/func-addr.ll
llvm/test/CodeGen/SPARC/globals.ll
llvm/test/CodeGen/SPARC/obj-relocs.ll
|