[PATCH] D86834: [InstSimplify] Protect against more poison in SimplifyWithOpReplaced (PR47322)

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: efriedma, aqjune, spatel, lebedev.ri.
Herald added subscribers: llvm-commits, danielkiss, hiraditya.
Herald added a project: LLVM.
nikic requested review of this revision.

Replace the check for poison-producing instructions in SimplifyWithOpReplaced() with the generic helper canCreatePoison() that properly handles poisonous shifts and thus avoids the problem from PR47322.

I should say that this is not really a full solution, more a minimal fix that is easy to backport to the release branch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86834

Files:
  llvm/lib/Analysis/InstructionSimplify.cpp
  llvm/test/Transforms/InstSimplify/select.ll


Index: llvm/test/Transforms/InstSimplify/select.ll
===================================================================
--- llvm/test/Transforms/InstSimplify/select.ll
+++ llvm/test/Transforms/InstSimplify/select.ll
@@ -941,3 +941,19 @@
   %spec.select  = and i1 %spec.select39, %spec.select40
   ret i1 %spec.select
 }
+
+define i32 @pr47322_more_poisonous_replacement(i32 %arg) {
+; CHECK-LABEL: @pr47322_more_poisonous_replacement(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[ARG:%.*]], 0
+; CHECK-NEXT:    [[TRAILING:%.*]] = call i32 @llvm.cttz.i32(i32 [[ARG]], i1 immarg true)
+; CHECK-NEXT:    [[SHIFTED:%.*]] = lshr i32 [[ARG]], [[TRAILING]]
+; CHECK-NEXT:    [[R1_SROA_0_1:%.*]] = select i1 [[CMP]], i32 0, i32 [[SHIFTED]]
+; CHECK-NEXT:    ret i32 [[R1_SROA_0_1]]
+;
+  %cmp = icmp eq i32 %arg, 0
+  %trailing = call i32 @llvm.cttz.i32(i32 %arg, i1 immarg true)
+  %shifted = lshr i32 %arg, %trailing
+  %r1.sroa.0.1 = select i1 %cmp, i32 0, i32 %shifted
+  ret i32 %r1.sroa.0.1
+}
+declare i32 @llvm.cttz.i32(i32, i1 immarg)
Index: llvm/lib/Analysis/InstructionSimplify.cpp
===================================================================
--- llvm/lib/Analysis/InstructionSimplify.cpp
+++ llvm/lib/Analysis/InstructionSimplify.cpp
@@ -3796,10 +3796,7 @@
     // TODO: This is an unusual limitation because better analysis results in
     //       worse simplification. InstCombine can do this fold more generally
     //       by dropping the flags. Remove this fold to save compile-time?
-    if (isa<OverflowingBinaryOperator>(B))
-      if (Q.IIQ.hasNoSignedWrap(B) || Q.IIQ.hasNoUnsignedWrap(B))
-        return nullptr;
-    if (isa<PossiblyExactOperator>(B) && Q.IIQ.isExact(B))
+    if (canCreatePoison(cast<Operator>(I)))
       return nullptr;
 
     if (MaxRecurse) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D86834.288766.patch
Type: text/x-patch
Size: 1792 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200829/ff03eae6/attachment.bin>