[PATCH] D24565: [InstCombine] PR30366 : Teach the udiv folding logic how to handle constant expressions.

[Andrea Di Biagio via llvm-commits]

andreadb created this revision.
andreadb added reviewers: spatel, RKSimon, uabelho.
andreadb added a subscriber: llvm-commits.

This patch fixes PR30366.

Function 'foldUDivShl' works under the assumption that the value in input to the function is always an llvm::Instruction.
However, function 'visitUDivOperand' (the only user of 'foldUDivShl') clearly violates that precondition since it uses pattern matchers to check if the udiv can be folded. Internally, pattern matchers for binary operators know how to work with both Instruction and ConstantExpr values.

This causes problems with instructions like this one:

udiv i32 %z, zext (i16 shl (i16 1, i16 ptrtoint ([1 x i16]* @b to i16)) to i32)

Where:
 Op1 = zext (i16 shl (i16 1, i16 ptrtoint ([1 x i16]* @b to i16)) to i32)

Internally, function 'visitUDivOperand' uses the following matcher:
  match(Op1, m_ZExt(m_Shl(m_Power2(), m_Value())))

In our example, that matcher would return 'true'. However, Op1 is a ConstantExpr and not an Instruction! Later on, Op1 is passed in input to foldUDivShl ; that function attempts to simplify the udiv instruction according to the following rule:
  // X udiv (C1 << N), where C1 is "1<<C2"  -->  X >> (N+C2)

The code in foldUDivShl  explicitly casts Op1 to llvm::Instruction. In our example, this would trigger an assertion failure due to an invalid cast from ConstantExpr to Instruction.

This patch fixes the problem in foldUDivShl() using pattern matchers instead of using explicit casts. The reduced test case from pr30366 has been added to test file InstCombine/udiv-simplify.ll.

Please let me know if okay to commit.

Thanks,
Andrea

https://reviews.llvm.org/D24565

Files:
  lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
  test/Transforms/InstCombine/udiv-simplify.ll

Index: test/Transforms/InstCombine/udiv-simplify.ll
===================================================================
--- test/Transforms/InstCombine/udiv-simplify.ll
+++ test/Transforms/InstCombine/udiv-simplify.ll
@@ -47,3 +47,18 @@
   %z = sext i32 %r to i64
   ret i64 %z
 }
+
+; The udiv should be simplified according to the rule:
+; X udiv (C1 << N), where C1 is `1<<C2` --> X >> (N+C2)
+ at b = external global [1 x i16]
+
+define i32 @PR30366(i1 %a) {
+; CHECK-LABEL: @PR30366(
+; CHECK-NEXT:    [[Z:%.*]] = zext i1 %a to i32
+; CHECK-NEXT:    [[D:%.*]] = lshr i32 [[Z]], zext (i16 ptrtoint ([1 x i16]* @b to i16) to i32)
+; CHECK-NEXT:    ret i32 [[D]]
+;
+  %z = zext i1 %a to i32
+  %d = udiv i32 %z, zext (i16 shl (i16 1, i16 ptrtoint ([1 x i16]* @b to i16)) to i32)
+  ret i32 %d
+}
Index: lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
+++ lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
@@ -993,17 +993,18 @@
 // X udiv (C1 << N), where C1 is "1<<C2"  -->  X >> (N+C2)
 static Instruction *foldUDivShl(Value *Op0, Value *Op1, const BinaryOperator &I,
                                 InstCombiner &IC) {
-  Instruction *ShiftLeft = cast<Instruction>(Op1);
-  if (isa<ZExtInst>(ShiftLeft))
-    ShiftLeft = cast<Instruction>(ShiftLeft->getOperand(0));
-
-  const APInt &CI =
-      cast<Constant>(ShiftLeft->getOperand(0))->getUniqueInteger();
-  Value *N = ShiftLeft->getOperand(1);
-  if (CI != 1)
-    N = IC.Builder->CreateAdd(N, ConstantInt::get(N->getType(), CI.logBase2()));
-  if (ZExtInst *Z = dyn_cast<ZExtInst>(Op1))
-    N = IC.Builder->CreateZExt(N, Z->getDestTy());
+  Value *ShiftLeft = nullptr;
+  if (!match(Op1, m_ZExt(m_Value(ShiftLeft))))
+    ShiftLeft = Op1;
+
+  const APInt *CI;
+  Value *N;
+  match(ShiftLeft, m_Shl(m_APInt(CI), m_Value(N)));
+  if (*CI != 1)
+    N = IC.Builder->CreateAdd(N, ConstantInt::get(N->getType(),
+                              CI->logBase2()));
+  if (Op1 != ShiftLeft)
+    N = IC.Builder->CreateZExt(N, Op1->getType());
   BinaryOperator *LShr = BinaryOperator::CreateLShr(Op0, N);
   if (I.isExact())
     LShr->setIsExact();


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D24565.71364.patch
Type: text/x-patch
Size: 2231 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160914/d8d7ef5d/attachment.bin>