[PATCH] D43276: [InstCombine] Don't fold select(C, Z, binop(select(C, X, Y), W)) -> select(C, Z, binop(Y, W)) if the binop is rem or div.

[Craig Topper via Phabricator via llvm-commits]

craig.topper created this revision.
craig.topper added reviewers: spatel, davide.

The select may have been preventing a division by zero so removing it might not be safe.

Fixes PR36362.


Repository:
  rL LLVM

https://reviews.llvm.org/D43276

Files:
  lib/Transforms/InstCombine/InstCombineSelect.cpp
  test/Transforms/InstCombine/pr36362.ll


Index: test/Transforms/InstCombine/pr36362.ll
===================================================================
--- /dev/null
+++ test/Transforms/InstCombine/pr36362.ll
@@ -0,0 +1,33 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+;RUN: opt -instcombine -S %s | FileCheck %s
+
+ at a = common local_unnamed_addr global i32 0, align 4
+ at b = common local_unnamed_addr global i32 0, align 4
+ at c = local_unnamed_addr global i32 -1, align 4
+
+; We shouldn't remove the select before the srem
+define i32 @foo() {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT:    [[ZERO:%.*]] = load i32, i32* @a, align 4
+; CHECK-NEXT:    [[X:%.*]] = load i32, i32* @b, align 4
+; CHECK-NEXT:    [[NEG1:%.*]] = load i32, i32* @c, align 4
+; CHECK-NEXT:    [[FALSE:%.*]] = icmp ne i32 [[ZERO]], 0
+; CHECK-NEXT:    [[Z:%.*]] = xor i32 [[NEG1]], -1
+; CHECK-NEXT:    [[SEL1:%.*]] = select i1 [[FALSE]], i32 [[Z]], i32 -1
+; CHECK-NEXT:    [[REM:%.*]] = srem i32 [[X]], [[SEL1]]
+; CHECK-NEXT:    [[SEL2:%.*]] = select i1 [[FALSE]], i32 [[REM]], i32 0
+; CHECK-NEXT:    store i32 [[SEL2]], i32* @b, align 4
+; CHECK-NEXT:    ret i32 0
+;
+  %zero = load i32, i32* @a, align 4
+  %x = load i32, i32* @b, align 4
+  %neg1 = load i32, i32* @c, align 4
+  %false = icmp ne i32 %zero, 0
+  %z = xor i32 %neg1, -1
+  %sel1 = select i1 %false, i32 %z, i32 -1
+  %rem = srem i32 %x, %sel1
+  %sel2 = select i1 %false, i32 %rem, i32 0
+  store i32 %sel2, i32* @b, align 4
+  ret i32 0
+}
+
Index: lib/Transforms/InstCombine/InstCombineSelect.cpp
===================================================================
--- lib/Transforms/InstCombine/InstCombineSelect.cpp
+++ lib/Transforms/InstCombine/InstCombineSelect.cpp
@@ -1760,11 +1760,25 @@
     }
   }
 
+  auto canMergeSelectThroughBinop = [](BinaryOperator *BO) {
+    // The select might be preventing a division by 0.
+    switch (BO->getOpcode()) {
+    default:
+      return true;
+    case Instruction::SRem:
+    case Instruction::URem:
+    case Instruction::SDiv:
+    case Instruction::UDiv:
+      return false;
+    }
+  };
+
   // Try to simplify a binop sandwiched between 2 selects with the same
   // condition.
   // select(C, binop(select(C, X, Y), W), Z) -> select(C, binop(X, W), Z)
   BinaryOperator *TrueBO;
-  if (match(TrueVal, m_OneUse(m_BinOp(TrueBO)))) {
+  if (match(TrueVal, m_OneUse(m_BinOp(TrueBO))) &&
+      canMergeSelectThroughBinop(TrueBO)) {
     if (auto *TrueBOSI = dyn_cast<SelectInst>(TrueBO->getOperand(0))) {
       if (TrueBOSI->getCondition() == CondVal) {
         TrueBO->setOperand(0, TrueBOSI->getTrueValue());
@@ -1783,7 +1797,8 @@
 
   // select(C, Z, binop(select(C, X, Y), W)) -> select(C, Z, binop(Y, W))
   BinaryOperator *FalseBO;
-  if (match(FalseVal, m_OneUse(m_BinOp(FalseBO)))) {
+  if (match(FalseVal, m_OneUse(m_BinOp(FalseBO))) &&
+      canMergeSelectThroughBinop(FalseBO)) {
     if (auto *FalseBOSI = dyn_cast<SelectInst>(FalseBO->getOperand(0))) {
       if (FalseBOSI->getCondition() == CondVal) {
         FalseBO->setOperand(0, FalseBOSI->getFalseValue());


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D43276.134159.patch
Type: text/x-patch
Size: 3072 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180214/c4637ce7/attachment.bin>