Fixes http://llvm.org/bugs/show_bug.cgi?id=20016 reproducible on new
lea-5.ll case.
Also use RSP/RBP for x32 lea to save 1 byte used for 0x67 prefix in
ESP/EBP case.
Details
Diff Detail
Event Timeline
LGTM with one nit.
test/CodeGen/X86/lea-4.ll | ||
---|---|---|
1 | this triple should probably still specify linux as the OS, as the others do. |
It's appeared that last version of patch is not correct because both needsStackRealignment and Opc == X86::LEA64_32r can be true at the same time. Making this check separate again and adding test revealing that.
Please review.
test/CodeGen/X86/lea-5.ll | ||
---|---|---|
6 | Unfortunately this won't work in environments without sed (none of the other tests use sed either). While it's not great, it's probably better just to duplicate this test function with the other alignment. (presumably then you can also de-regex the -40 in the CHECK line?) |
Ok. Actually I found couple of tests with sed (e.g. test/Transforms/GCOVProfiling/linezero.ll) so thought it's fine... not sure how they work though when no sed available. I'll change this test.
thanks; yeah I wasn't sure either. I asked on IRC and the consensus seemed to be to just duplicate if the test is fairly small.
test/CodeGen/X86/lea-5.ll | ||
---|---|---|
48 | shouldn't this just be X32: ? |
test/CodeGen/X86/lea-5.ll | ||
---|---|---|
48 | It's called at line 35, but you are right, it's redundant. |
should this just be added to the if-else chain above?