llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.03k stars 11.58k forks source link

PowerPC misoptimization for scalar float -> vector float load #25531

Closed llvmbot closed 8 years ago

llvmbot commented 8 years ago
Bugzilla Link 25157
Resolution FIXED
Resolved on Oct 14, 2015 15:47
Version 3.7
OS Linux
Reporter LLVM Bugzilla Contributor
CC @echristo,@hfinkel

Extended Description

In PPCDAGToDagISel::Select() under the case ISD::VECTOR_SHUFFLE, there is an optimization to convert a shuffle into a LXVDSX instruction (Load VSX Vector Doubleword, Splat Indexed). This works great when the values in memory is actually a doubleword. Unfortunately, it produces incorrect results when the value is only, say, a word in length and to be converted to a double after the load.

Here is a possible fix: @@ -2786,7 +2786,7 @@ SDNode PPCDAGToDAGISel::Select(SDNode N) { LoadSDNode *LD = cast(Op1.getOperand(0)); SDValue Base, Offset;

llvmbot commented 8 years ago

Fixed the correctness issue as r250324. I'll have a look at the poor code quality separately.

llvmbot commented 8 years ago

I'll take this one.

llvmbot commented 8 years ago

The xxswapd is part of the store (endianness correction), but the fact that we have a redundant splat there is just ... awesome. Obviously the swap is never needed after a splat, also. We may need some vector peepholes to clean up such silliness, as this is probably not the only place such things pop up.

llvmbot commented 8 years ago

reduced.ll Here is the test case. I am able to tickle this optimization (setting an appropriate breakpoint) with:

gdb --args llc -O2 reduced.ll

llvmbot commented 8 years ago

Yes. More work is needed. After this correctness fix, the code sequence emitted is:

lxsspx 0, 23, 3
xxspltd  1, 0, 0
xxspltd  1, 1, 1
xxswapd  1, 1
llvmbot commented 8 years ago

I would think MVT::i64 would be fine also. Do you have a test case?