These test cases generate slightly different code sequences when VSX is activated and thus fail. The update turns off VSX explicitly for the existing checks and then adds a second set of checks for most of them that test the VSX instruction output.
Diff Detail
Event Timeline
The patch itself is ok, but when I apply it to current trunk it exposes a crash in the VSX FMA mutation pass. If you can reproduce, please open a bug at llvm.org/bugs (against libraries/Backend:PowerPC). Crash follows:
/home/wschmidt/llvm/build/llvm-test/Release+Asserts/bin/llc < /home/wschmidt/ll\
vm/llvm-test/test/CodeGen/PowerPC/recipest.ll -mtriple=powerpc64-unknown-linux-\
gnu -mcpu=pwr7 -mattr=+vsx | /home/wschmidt/llvm/build/llvm-test/Release+Assert\
s/bin/FileCheck -check-prefix=CHECK-SAFE-VSX /home/wschmidt/llvm/llvm-test/test\
/CodeGen/PowerPC/recipest.ll
Exit Code: 2
Command Output (stderr):
llc: /home/wschmidt/llvm/llvm-test/lib/CodeGen/LiveInterval.cpp:299: llvm::Live\
Range::Segment* llvm::LiveRange::addSegmentFrom(llvm::LiveRange::Segment, llvm:\
:LiveRange::iterator): Assertion `B->end <= Start && "Cannot overlap two segmen\
ts with differing ValID's" " (did you def the same reg twice in a MachineInstr?\
)"' failed.
#0 0x1135fbac llvm::sys::PrintStackTrace(_IO_FILE*) (/home/wschmidt/llvm/build/\
llvm-test/Release+Asserts/bin/llc+0x1135fbac)
#1 0x1135fe44 PrintStackTraceSignalHandler(void*) (/home/wschmidt/llvm/build/ll\
vm-test/Release+Asserts/bin/llc+0x1135fe44)
#2 0x1135d754 SignalHandler(int) (/home/wschmidt/llvm/build/llvm-test/Release+A\
sserts/bin/llc+0x1135d754)
0 llc 0x000000001135fbac llvm::sys::PrintStackTrace(_IO_FILE*) + 92
1 llc 0x000000001135fe44
2 llc 0x000000001135d754
3 0x00003fff8fd00478 kernel_sigtramp_rt64 + 0
4 libc.so.6 0x00003fff8f820a88 gsignal + 72
5 libc.so.6 0x00003fff8f82693c abort + 620
6 libc.so.6 0x00003fff8f8165b4
7 libc.so.6 0x00003fff8f8166a4 assert_fail + 100
8 llc 0x0000000010c709f0 llvm::LiveRange::addSegmentFrom(llvm::LiveRange\
::Segment, llvm::LiveRange::Segment*) + 912
9 llc 0x0000000010845f34
10 llc 0x0000000010cdd178 llvm::MachineFunctionPass::runOnFunction(llvm::\
Function&) + 280
11 llc 0x00000000112cd880 llvm::FPPassManager::runOnFunction(llvm::Functi\
on&) + 640
12 llc 0x00000000112cdeec llvm::FPPassManager::runOnModule(llvm::Module&)\
+ 76
13 llc 0x00000000112ce308 llvm::legacy::PassManagerImpl::run(llvm::Module\
&) + 968
14 llc 0x00000000112ce5bc llvm::legacy::PassManager::run(llvm::Module&) +\
28
15 llc 0x0000000010243408
16 llc 0x000000001021bb10 main + 528
17 libc.so.6 0x00003fff8f804d00
18 libc.so.6 0x00003fff8f804ef8 __libc_start_main + 200
Stack dump:
0. Program arguments: /home/wschmidt/llvm/build/llvm-test/Release+Asserts/\
bin/llc -mtriple=powerpc64-unknown-linux-gnu -mcpu=pwr7 -enable-unsafe-fp-math \
-mattr=+vsx
- Running pass 'Function Pass Manager' on module '<stdin>'.
- Running pass 'PowerPC VSX FMA Mutation' on function '@foo3'
FileCheck error: '-' is empty.
My recommendation would be to leave the recipest.ll test out of this patch for now, so the buildbots stay green. Instead, include the changes in the bugzilla so it can be added back when the bug is fixed.
This version LGTM and passes my testing. If no objections arise in the meantime, I'll commit this on seurer's behalf around 10 p.m. Central tonight.
I gather the reason for this patch is to increase the amount of testing that vsx gets while not just reusing the existing testcases and not writing new ones?
One small nit:
/home/seurer/llvm/llvm-oneoff/test/CodeGen/PowerPC/vec_mul.ll | ||
---|---|---|
92 | Newline. |
The reason for the test is to correct the tests that generate different code when VSX is enabled (usually a vector-scalar version of the floating-point instruction, which has identical behavior but allows selection from 64 floating-point registers instead of 32). Hal already added quite a large number of VSX tests, and these complement those.
I should have commented on the newline -- I corrected that when I was testing his patch, so when i commit it that will be fixed.
OK, so the traditional way to do that would have been to just use a -mtriple on the command line to specify the exact machine you wanted to target rather than just using llc. If you want to test more code generation then this is fine, but if it's just random testing without purpose then I don't know if you want to keep it or not.
Except that currently there are no triples that generate VSX code because VSX code is disabled. The specific use of -mattr=+/-vsx allows us to get closer to enabling VSX, which is the end goal (moving target) of these patches. All of these incorrectly failing tests make it difficult to spot the actual problems. This seems the best way forward, and the extra VSX testing is a bonus.
The way to do that would, canonically, be -mtriple -mattr (similar to what you've been doing). It's just whether or not you want to add new tests for vsx specific things or not. I have no objections to this though.
Newline.