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

[Sanjay Patel via llvm-commits]

Author: spatel
Date: Mon Aug  5 04:27:07 2019
New Revision: 367834

URL: http://llvm.org/viewvc/llvm-project?rev=367834&view=rev
Log:
[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.

Modified:
    llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
    llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
    llvm/trunk/test/CodeGen/X86/trunc-and.ll

Modified: llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp?rev=367834&r1=367833&r2=367834&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (original)
+++ llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp Mon Aug  5 04:27:07 2019
@@ -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).

Modified: llvm/trunk/lib/Target/X86/X86ISelLowering.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86ISelLowering.cpp?rev=367834&r1=367833&r2=367834&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86ISelLowering.cpp (original)
+++ llvm/trunk/lib/Target/X86/X86ISelLowering.cpp Mon Aug  5 04:27:07 2019
@@ -28846,8 +28846,6 @@ bool X86TargetLowering::isLegalStoreImme
 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;

Modified: llvm/trunk/test/CodeGen/X86/trunc-and.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/trunc-and.ll?rev=367834&r1=367833&r2=367834&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/X86/trunc-and.ll (original)
+++ llvm/trunk/test/CodeGen/X86/trunc-and.ll Mon Aug  5 04:27:07 2019
@@ -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] }
+
+ at 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
+}