[PATCH] D44097: [ARM] Fix for PR36577

[Sam Parker via Phabricator via llvm-commits]

samparker created this revision.
samparker added a reviewer: dim.
Herald added subscribers: kristof.beyls, javed.absar.

Don't PerformSHLSimplify if the given node is used by a node that also uses a constant because we may get stuck in an infinite combine loop.

bugzilla: https://bugs.llvm.org/show_bug.cgi?id=36577


https://reviews.llvm.org/D44097

Files:
  lib/Target/ARM/ARMISelLowering.cpp
  test/CodeGen/ARM/pr36577.ll


Index: test/CodeGen/ARM/pr36577.ll
===================================================================
--- /dev/null
+++ test/CodeGen/ARM/pr36577.ll
@@ -0,0 +1,28 @@
+; RUN: llc -mtriple armv6t2 %s -o - | FileCheck %s
+; RUN: llc -mtriple thumbv6t2 %s -o - | FileCheck %s --check-prefix=CHECK-T2
+; RUN: llc -mtriple armv7 %s -o - | FileCheck %s
+; RUN: llc -mtriple thumbv7 %s -o - | FileCheck %s --check-prefix=CHECK-T2
+; RUN: llc -mtriple thumbv7m %s -o - | FileCheck %s --check-prefix=CHECK-T2
+; RUN: llc -mtriple thumbv8m.main %s -o - | FileCheck %s --check-prefix=CHECK-T2
+
+ at a = common dso_local local_unnamed_addr global i16 0, align 2
+
+; CHECK-LABEL: pr36577
+; CHECK: ldrh r0, [r0]
+; CHECK: bic r0, r1, r0, lsr #5
+; CHECK: mvn r1, #7
+; CHECK: orr r0, r0, r1
+; CHECK-T2: ldrh r0, [r0]
+; CHECK-T2: bic.w r0, r1, r0, lsr #5
+; CHECK-T2: orn r0, r0, #7
+define dso_local arm_aapcscc i32** @pr36577() {
+entry:
+  %0 = load i16, i16* @a, align 2
+  %1 = lshr i16 %0, 7
+  %2 = and i16 %1, 1
+  %3 = zext i16 %2 to i32
+  %4 = xor i32 %3, -1
+  %add.ptr = getelementptr inbounds i32*, i32** null, i32 %4
+  ret i32** %add.ptr
+}
+
Index: lib/Target/ARM/ARMISelLowering.cpp
===================================================================
--- lib/Target/ARM/ARMISelLowering.cpp
+++ lib/Target/ARM/ARMISelLowering.cpp
@@ -10442,7 +10442,14 @@
     case ISD::XOR:
     case ISD::SETCC:
     case ARMISD::CMP:
-      // Check that its not already using a shl.
+      // Check that the user isn't already using a constant because there
+      // aren't any instructions that support an immediate operand and a
+      // shifted operand.
+      if (isa<ConstantSDNode>(U->getOperand(0)) ||
+          isa<ConstantSDNode>(U->getOperand(1)))
+        return SDValue();
+
+      // Check that its not already using a shift.
       if (U->getOperand(0).getOpcode() == ISD::SHL ||
           U->getOperand(1).getOpcode() == ISD::SHL)
         return SDValue();
@@ -10464,8 +10471,6 @@
   if (!C1ShlC2 || !C2)
     return SDValue();
 
-  DEBUG(dbgs() << "Trying to simplify shl: "; N->dump());
-
   APInt C2Int = C2->getAPIntValue();
   APInt C1Int = C1ShlC2->getAPIntValue();
 
@@ -10479,12 +10484,12 @@
   C1Int.lshrInPlace(C2Int);
 
   // The immediates are encoded as an 8-bit value that can be rotated.
-  unsigned Zeros = C1Int.countLeadingZeros() + C1Int.countTrailingZeros();
-  if (C1Int.getBitWidth() - Zeros > 8)
-    return SDValue();
+  auto LargeImm = [](const APInt &Imm) {
+    unsigned Zeros = Imm.countLeadingZeros() + Imm.countTrailingZeros();
+    return Imm.getBitWidth() - Zeros > 8;
+  };
 
-  Zeros = C2Int.countLeadingZeros() + C2Int.countTrailingZeros();
-  if (C2Int.getBitWidth() - Zeros > 8)
+  if (LargeImm(C1Int) || LargeImm(C2Int))
     return SDValue();
 
   SelectionDAG &DAG = DCI.DAG;
@@ -10495,6 +10500,10 @@
   // Shift left to compensate for the lshr of C1Int.
   SDValue Res = DAG.getNode(ISD::SHL, dl, MVT::i32, BinOp, SHL.getOperand(1));
 
+  DEBUG(dbgs() << "Simplify shl use:\n"; SHL.getOperand(0).dump(); SHL.dump();
+        N->dump());
+  DEBUG(dbgs() << "Into:\n"; X.dump(); BinOp.dump(); Res.dump());
+
   DAG.ReplaceAllUsesWith(SDValue(N, 0), Res);
   return SDValue(N, 0);
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D44097.137011.patch
Type: text/x-patch
Size: 3232 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180305/9c1019fe/attachment.bin>