[X86] Don't peek through bitcasts before checking ISD::isBuildVectorOfConstantSDNodes in combineTruncatedArithmetic

[craig.topper]

Summary

We don't have any combines that can look through a bitcast to truncate a build vector of constants. So the truncate will stick around and give us something like this pattern (binop (trunc X), (trunc (bitcast (build_vector)))) which has two truncates in it. Which will be reversed by hoistLogicOpWithSameOpcodeHands in the generic DAG combiner. Thus causing an infinite loop.

Even if we had a combine for (truncate (bitcast (build_vector))), I think it would need to be implemented in getNode otherwise DAG combiner visit ordering would probably still visit the binop first and reverse it. Or combineTruncatedArithmetic would need to do its own constant folding.

This came to me as infinite loop a colleague found with ISPC. In his case the bitcast got introduced after type legalization do it being a v4i64 build_vector on a 32-bit target. But that also means you need to have delayed combineTruncatedArithmetic seeing the constant until after type legalization. So my early attempts at a simple test case have failed. I'll keep at it.

[craig.topper]

Compiling this with "-mattr=avx2 -mtriple=i686-unknown-unknown" should be enough to hit the bug. I'll get that turned into a real test case later tonight.

define <8 x i32> @foo(<8 x i64> %x, <4 x i64> %y) {
  %a = shufflevector <4 x i64> %y, <4 x i64> <i64 12345, i64 67890, i64 13579, i64 24680>, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
  %b = and <8 x i64> %x, %a
  %c = trunc <8 x i64> %b to <8 x i32>
  ret <8 x i32> %c
}

[craig.topper]

Add test case

[spatel]

LGTM

lib/Target/X86/X86ISelLowering.cpp

38719

Please add to this comment to make it explicit that we're not peeking through bitcast to avoid fighting with another transform.

lib/Target/X86/X86ISelLowering.cpp

auto IsFreeTruncation = [VT](SDValue Op) {
     // See if this has been extended from a smaller/equal size to
     // the truncation size, allowing a truncation to combine with the extend.
     unsigned Opcode = Op.getOpcode();
     if ((Opcode == ISD::ANY_EXTEND || Opcode == ISD::SIGN_EXTEND ||
          Opcode == ISD::ZERO_EXTEND) &&
         Op.getOperand(0).getScalarValueSizeInBits() <= TruncSizeInBits)
       return true;
 
     // See if this is a single use constant which can be constant folded.
-    SDValue BC = peekThroughOneUseBitcasts(Op);
-    return ISD::isBuildVectorOfConstantSDNodes(BC.getNode());
+    return ISD::isBuildVectorOfConstantSDNodes(Op.getNode());
   };
 
   auto TruncateArithmetic = [&](SDValue N0, SDValue N1) {
     SDValue Trunc0 = DAG.getNode(ISD::TRUNCATE, DL, VT, N0);
     SDValue Trunc1 = DAG.getNode(ISD::TRUNCATE, DL, VT, N1);
     return DAG.getNode(Opcode, DL, VT, Trunc0, Trunc1);
   };
 

test/CodeGen/X86/pr40891.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=i686-unknown-unknown -mattr=avx2 | FileCheck %s
+
+; Make sure this sequence doesn't hang in DAG combine.
+
+define <8 x i32> @foo(<8 x i64> %x, <4 x i64> %y) {
+; CHECK-LABEL: foo:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    vandps %ymm2, %ymm0, %ymm0
+; CHECK-NEXT:    vandps {{\.LCPI.*}}, %ymm1, %ymm1
+; CHECK-NEXT:    vextractf128 $1, %ymm0, %xmm2
+; CHECK-NEXT:    vshufps {{.*#+}} xmm0 = xmm0[0,2],xmm2[0,2]
+; CHECK-NEXT:    vextractf128 $1, %ymm1, %xmm2
+; CHECK-NEXT:    vshufps {{.*#+}} xmm1 = xmm1[0,2],xmm2[0,2]
+; CHECK-NEXT:    vinsertf128 $1, %xmm1, %ymm0, %ymm0
+; CHECK-NEXT:    retl
+  %a = shufflevector <4 x i64> %y, <4 x i64> <i64 12345, i64 67890, i64 13579, i64 24680>, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
+  %b = and <8 x i64> %x, %a
+  %c = trunc <8 x i64> %b to <8 x i32>
+  ret <8 x i32> %c
+}
+