[DAGCombiner][x86] prevent infinite loop from truncate/extend transforms

The test case is based on the example from the post-commit thread for:
https://reviews.llvm.org/rGc9171bd0a955

This replaces the x86-specific simple-type check from:
rL367766
with a check in the DAGCombiner. Adding the check isn't
strictly necessary after the fix from:
rL367768
...but it seems likely that we're heading for trouble if
we are creating weird types in this transform.

I combined the earlier legality check into the initial
clause to simplify the code.

So we should only try the trunc/sext transform at the
earliest combine stage, but we limit the transform to
simple types anyway because the TLI hook is probably
too lax about what it considers a free truncate.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@367834 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index f6b021c8fd1b3b304dd3bc709cc35991ec8afa93..77bc37ca5e98b7a453e38baea63c8a6c8962f8db 100644 (file)
--- a/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -7619,7 +7619,7 @@ SDValue DAGCombiner::visitSRA(SDNode *N) {
   // We convert trunc/ext to opposing shifts in IR, but casts may be cheaper.
   //   sra (add (shl X, N1C), AddC), N1C -->
   //   sext (add (trunc X to (width - N1C)), AddC')
-  if (!LegalOperations && N0.getOpcode() == ISD::ADD && N0.hasOneUse() && N1C &&
+  if (!LegalTypes && N0.getOpcode() == ISD::ADD && N0.hasOneUse() && N1C &&
       N0.getOperand(0).getOpcode() == ISD::SHL &&
       N0.getOperand(0).getOperand(1) == N1 && N0.getOperand(0).hasOneUse()) {
     if (ConstantSDNode *AddC = isConstOrConstSplat(N0.getOperand(1))) {
@@ -7631,7 +7631,12 @@ SDValue DAGCombiner::visitSRA(SDNode *N) {
       EVT TruncVT = EVT::getIntegerVT(Ctx, OpSizeInBits - ShiftAmt);
       if (VT.isVector())
         TruncVT = EVT::getVectorVT(Ctx, TruncVT, VT.getVectorNumElements());
-      if (isTypeLegal(TruncVT) && TLI.isTruncateFree(VT, TruncVT)) {
+
+      // TODO: The simple type check probably belongs in the default hook
+      //       implementation and/or target-specific overrides (because
+      //       non-simple types likely require masking when legalized), but that
+      //       restriction may conflict with other transforms.
+      if (TruncVT.isSimple() && TLI.isTruncateFree(VT, TruncVT)) {
         SDLoc DL(N);
         SDValue Trunc = DAG.getZExtOrTrunc(Shl.getOperand(0), DL, TruncVT);
         SDValue ShiftC = DAG.getConstant(AddC->getAPIntValue().lshr(ShiftAmt).

diff --git a/lib/Target/X86/X86ISelLowering.cpp b/lib/Target/X86/X86ISelLowering.cpp
index b4473ef67372392568f67d1c781b8b3f8d4051c1..899c6afa7d06092cd29e9825fdc048c698dcda93 100644 (file)
--- a/lib/Target/X86/X86ISelLowering.cpp
+++ b/lib/Target/X86/X86ISelLowering.cpp
@@ -28846,8 +28846,6 @@ bool X86TargetLowering::isLegalStoreImmediate(int64_t Imm) const {
 bool X86TargetLowering::isTruncateFree(EVT VT1, EVT VT2) const {
   if (!VT1.isInteger() || !VT2.isInteger())
     return false;
-  if (!VT1.isSimple() || !VT2.isSimple())
-    return false;
   unsigned NumBits1 = VT1.getSizeInBits();
   unsigned NumBits2 = VT2.getSizeInBits();
   return NumBits1 > NumBits2;

diff --git a/test/CodeGen/X86/trunc-and.ll b/test/CodeGen/X86/trunc-and.ll
index 28a55c257bdbfc8788c8c8ddfd16c793c4111e98..09fe6413d27a5169a255d936b83710b166fb3036 100644 (file)
--- a/test/CodeGen/X86/trunc-and.ll
+++ b/test/CodeGen/X86/trunc-and.ll
@@ -24,3 +24,27 @@ define i16 @PR40793(<8 x i16> %t1) {
 
 declare <2 x double> @llvm.fabs.v2f64(<2 x double>)
 
+; This would infinite loop by trying to truncate and any_extend.
+
+%struct.anon = type { [9 x i8], [3 x i8] }
+
+@b = common local_unnamed_addr global %struct.anon zeroinitializer, align 4
+
+define i32 @d() {
+; CHECK-LABEL: d:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    movzbl b+{{.*}}(%rip), %ecx
+; CHECK-NEXT:    andl $7, %ecx
+; CHECK-NEXT:    movl $d, %eax
+; CHECK-NEXT:    addl %ecx, %eax
+; CHECK-NEXT:    # kill: def $eax killed $eax killed $rax
+; CHECK-NEXT:    retq
+  %bf.load = load i72, i72* bitcast (%struct.anon* @b to i72*), align 4
+  %bf.lshr = lshr i72 %bf.load, 64
+  %t0 = trunc i72 %bf.lshr to i64
+  %bf.cast = and i64 %t0, 7
+  %add.ptr = getelementptr i8, i8* bitcast (i32 ()* @d to i8*), i64 %bf.cast
+  %t1 = ptrtoint i8* %add.ptr to i64
+  %t2 = trunc i64 %t1 to i32
+  ret i32 %t2
+}