This is an archive of the discontinued LLVM Phabricator instance.

LLVM codegen test cases updated to check instructions generated when VSX is activated on Power
ClosedPublic

Authored by seurer on Oct 13 2014, 12:57 PM.

Details

Summary

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

seurer updated this revision to Diff 14814.Oct 13 2014, 12:57 PM
seurer retitled this revision from to [patch] LLVM codegen test cases updated to check instructions generated with VSX activated on Power.
seurer updated this object.
seurer edited the test plan for this revision. (Show Details)
seurer retitled this revision from [patch] LLVM codegen test cases updated to check instructions generated with VSX activated on Power to [patch] LLVM codegen test cases updated to check instructions generated when VSX is activated on Power.Oct 14 2014, 8:06 AM
seurer added reviewers: echristo, hfinkel, wschmidt.
seurer added a subscriber: Unknown Object (MLST).
seurer retitled this revision from [patch] LLVM codegen test cases updated to check instructions generated when VSX is activated on Power to LLVM codegen test cases updated to check instructions generated when VSX is activated on Power.Oct 14 2014, 12:35 PM
wschmidt edited edge metadata.Oct 14 2014, 6:27 PM

This patch LGTM. Eric, any further concerns?

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

  1. Running pass 'Function Pass Manager' on module '<stdin>'.
  2. Running pass 'PowerPC VSX FMA Mutation' on function '@foo3'

FileCheck error: '-' is empty.

The crash was obtained with r219765 and your patch applied.

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.

seurer updated this revision to Diff 14958.Oct 15 2014, 1:16 PM
seurer edited edge metadata.

Removed CodeGen/PowerPC/recipest.ll

wschmidt accepted this revision.Oct 16 2014, 4:30 PM
wschmidt edited edge metadata.

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.

This revision is now accepted and ready to land.Oct 16 2014, 4:30 PM
echristo edited edge metadata.Oct 16 2014, 4:33 PM

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.

echristo accepted this revision.Oct 16 2014, 4:55 PM
echristo edited edge metadata.

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.

Committed as r220019. This can be closed now.

seurer closed this revision.Dec 8 2014, 12:46 PM

Closing. Was committed as r220019.