[SelectionDAG] Unary vector constant folding integer legality fixes

[RKSimon]

Summary

This patch fixes issues with vector constant folding not correctly handling scalar input operands if they require implicit truncation - this was tested with llvm-stress as recommended by Patrik.

The patch ensures that integer input scalars from a build vector are correctly truncated before folding, and that constant integer scalar results are promoted to a legal type before inclusion in the new folded build vector. I've ran llvm-stress for extended lengths of time without any issue due to the vector constant folding path.

I have added another crash test case and also a test for UINT_TO_FP / SINT_TO_FP using an non-truncated scalar input, which was failing before this patch.

[silvas]

We probably don't want to directly commit the test case generated by llvm-stress. Can you reduce it? These are theoretically the sorts of things that bugpoint should reduce really well, although I've always found it hard to use.

http://llvm.org/docs/CommandGuide/bugpoint.html
http://llvm.org/docs/Bugpoint.html

[RKSimon]

Reduced the llvm-stress crash case with bugpoint - thanks for the reminder Sean.

[ab]

Is there a way to further reduce the testcase, or even rewrite it from scratch? I shiver whenever I see autogen ;)

The change LGTM, but you might want to wait for other reviews.

test/CodeGen/X86/fold-vector-bv-crash.ll

2–3

cpu -> attr?

test/CodeGen/X86/fold-vector-trunc-sitofp.ll

2–3

Isn't SSE2 enough?

[RKSimon]

Cleaned up crash test (to remove the autogen stink!)

[RKSimon]

In D9282#162777, @ab wrote:

Is there a way to further reduce the testcase, or even rewrite it from scratch? I shiver whenever I see autogen ;)

The change LGTM, but you might want to wait for other reviews.

Thanks Ahmed, I'll give it another day to see if anyone else has any comments.

I'll get the test -mcpu fixed - note we need mattr=+avx to force i686 to use xmm registers, otherwise it does a rather messy return of the <4 x float> over the stack which isn't very helpful.

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

SDValue SelectionDAG::getNode(unsigned Opcode, SDLoc DL,
   // Constant fold unary operations with a vector integer or float operand.
   if (BuildVectorSDNode *BV = dyn_cast<BuildVectorSDNode>(Operand.getNode())) {
     if (BV->isConstant()) {
       switch (Opcode) {
       default:
         // FIXME: Entirely reasonable to perform folding of other unary
         // operations here as the need arises.
         break;
-      case ISD::TRUNCATE:
-        // Constant build vector truncation can be done with the original scalar
-        // operands but with a new build vector with the truncated value type.
-        return getNode(ISD::BUILD_VECTOR, DL, VT, BV->ops());
       case ISD::FNEG:
       case ISD::FABS:
       case ISD::FCEIL:
       case ISD::FTRUNC:
       case ISD::FFLOOR:
       case ISD::FP_EXTEND:
+      case ISD::TRUNCATE:
       case ISD::UINT_TO_FP:
       case ISD::SINT_TO_FP: {
+        EVT SVT = VT.getScalarType();
+        EVT InVT = BV->getValueType(0);
+        EVT InSVT = InVT.getScalarType();
+
+        // Find legal integer scalar type for constant promotion.
+        EVT LegalSVT = SVT;
+        if (SVT.isInteger()) {
+          LegalSVT = TLI->getTypeToTransformTo(*getContext(), SVT);
+          assert(LegalSVT.bitsGE(SVT) && "Unexpected legal scalar type size");
+        }
+
         // Let the above scalar folding handle the folding of each element.
         SmallVector<SDValue, 8> Ops;
         for (int i = 0, e = VT.getVectorNumElements(); i != e; ++i) {
           SDValue OpN = BV->getOperand(i);
-          OpN = getNode(Opcode, DL, VT.getVectorElementType(), OpN);
+          EVT OpVT = OpN.getValueType();
+
+          // Build vector (integer) scalar operands may need implicit
+          // truncation - do this before constant folding.
+          if (OpVT.isInteger() && OpVT.bitsGT(InSVT))
+            OpN = getNode(ISD::TRUNCATE, DL, InSVT, OpN);
+
+          OpN = getNode(Opcode, DL, SVT, OpN);
           if (OpN.getOpcode() != ISD::UNDEF &&
               OpN.getOpcode() != ISD::Constant &&
               OpN.getOpcode() != ISD::ConstantFP)
             break;
+
+          // Legalize the (integer) scalar constant if necessary.
+          if (LegalSVT != SVT)
+            OpN = getNode(ISD::ANY_EXTEND, DL, LegalSVT, OpN);
           Ops.push_back(OpN);
         }
         if (Ops.size() == VT.getVectorNumElements())
           return getNode(ISD::BUILD_VECTOR, DL, VT, Ops);
         break;
       }
       }
     }

test/CodeGen/X86/fold-vector-bv-crash.ll

+; RUN: llc < %s -mtriple=i686-unknown -mcpu=corei7
+; RUN: llc < %s -mtriple=x86_64-unknown -mcpu=corei7
+
+;
+; llvm-stress generated crash case due to build_vector implicit
+; truncation bug from constant folding after legalization.
+;
+
+@G = external global i32
+
+define void @bv_crash_test() {
+  %I = insertelement <4 x i64> zeroinitializer, i64 15910, i32 0
+  %Tr = trunc <4 x i64> %I to <4 x i8>
+  %Bc = bitcast <4 x i8> %Tr to i32
+  store volatile i32 %Bc, i32* @G
+  ret void
+}

test/CodeGen/X86/fold-vector-trunc-sitofp.ll

+; RUN: llc < %s -mtriple=i686-unknown -mattr=+avx | FileCheck %s
+; RUN: llc < %s -mtriple=x86_64-unknown -mattr=+avx | FileCheck %s
+
+; Check that constant integer correctly being truncated before float conversion
+
+define <4 x float> @test1() {
+; CHECK-LABEL: test1
+; CHECK: vmovaps {{.*#+}} xmm0 = [-1.000000e+00,0.000000e+00,-1.000000e+00,0.000000e+00]
+; CHECK-NEXT: ret
+  %1 = trunc <4 x i3> <i3 -1, i3 -22, i3 7, i3 8> to <4 x i1>
+  %2 = sitofp <4 x i1> %1 to <4 x float>
+  ret <4 x float> %2
+}