[PATCH] D87965: [InstCombine] replace phi values from unreachable blocks with 'undef'

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
spatel added reviewers: nikic, lebedev.ri, aqjune, jdoerfert, uabelho.
Herald added subscribers: hiraditya, mcrosier.
Herald added a project: LLVM.
spatel requested review of this revision.

The test (currently crashing) is reduced from the example provided in the post-commit discussion in D87149 <https://reviews.llvm.org/D87149>.
Here we replace with undef because we can't remove the operand completely (because InstCombine preserves the CFG).
We used to do something similar for branches, but we changed that to use a fixed constant value rather than undef because branching on undef became poison/UB.

I'm not sure what the cost/implications of this approach are. If we want to fix the crash more directly, we could just add a clause for uses and restrict the transform from D87149 <https://reviews.llvm.org/D87149>.


https://reviews.llvm.org/D87965

Files:
  llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
  llvm/test/Transforms/InstCombine/phi.ll
  llvm/test/Transforms/InstCombine/zext-or-icmp.ll


Index: llvm/test/Transforms/InstCombine/zext-or-icmp.ll
===================================================================
--- llvm/test/Transforms/InstCombine/zext-or-icmp.ll
+++ llvm/test/Transforms/InstCombine/zext-or-icmp.ll
@@ -31,14 +31,7 @@
 ; CHECK:       block1:
 ; CHECK-NEXT:    br label [[BLOCK2]]
 ; CHECK:       block2:
-; CHECK-NEXT:    [[CMP_I:%.*]] = phi i1 [ false, [[BLOCK1:%.*]] ], [ true, [[ENTRY:%.*]] ]
-; CHECK-NEXT:    [[M_011:%.*]] = phi i32 [ 0, [[BLOCK1]] ], [ 33, [[ENTRY]] ]
-; CHECK-NEXT:    [[M_1_OP:%.*]] = lshr i32 1, [[M_011]]
-; CHECK-NEXT:    [[SEXT_MASK:%.*]] = and i32 [[M_1_OP]], 65535
-; CHECK-NEXT:    [[CMP115:%.*]] = icmp ne i32 [[SEXT_MASK]], 0
-; CHECK-NEXT:    [[CMP1:%.*]] = or i1 [[CMP_I]], [[CMP115]]
-; CHECK-NEXT:    [[CONV2:%.*]] = zext i1 [[CMP1]] to i32
-; CHECK-NEXT:    ret i32 [[CONV2]]
+; CHECK-NEXT:    ret i32 1
 ;
 entry:
   br label %block2
Index: llvm/test/Transforms/InstCombine/phi.ll
===================================================================
--- llvm/test/Transforms/InstCombine/phi.ll
+++ llvm/test/Transforms/InstCombine/phi.ll
@@ -1181,3 +1181,39 @@
   %cmp1 = icmp ne i32 %a.0, 0
   ret i1  %cmp1
 }
+
+; This would crash trying to delete an instruction (conv)
+; that still had uses because the user (the phi) was not
+; updated to remove a use from an unreachable block (g.exit).
+
+define void @main(i16 %x) {
+; CHECK-LABEL: @main(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[FOR_COND:%.*]]
+; CHECK:       for.cond:
+; CHECK-NEXT:    br i1 undef, label [[FOR_END:%.*]], label [[FOR_BODY:%.*]]
+; CHECK:       for.body:
+; CHECK-NEXT:    unreachable
+; CHECK:       g.exit:
+; CHECK-NEXT:    br label [[FOR_COND]]
+; CHECK:       for.end:
+; CHECK-NEXT:    ret void
+;
+entry:
+  br label %for.cond
+
+for.cond:
+  %p = phi double [ %conv, %g.exit ], [ undef, %entry ]
+  br i1 undef, label %for.end, label %for.body
+
+for.body:
+  %conv = sitofp i16 %x to double
+  unreachable
+
+g.exit:
+  br label %for.cond
+
+for.end:
+  store double %p, double* undef
+  ret void
+}
Index: llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
===================================================================
--- llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
+++ llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp
@@ -1300,6 +1300,12 @@
   if (Instruction *Result = foldPHIArgZextsIntoPHI(PN))
     return Result;
 
+  // Replace values from unreachable incoming blocks with undef.
+  for (unsigned i = 0, e = PN.getNumIncomingValues(); i != e; ++i) {
+    if (!DT.isReachableFromEntry(PN.getIncomingBlock(i)))
+      PN.setIncomingValue(i, UndefValue::get(PN.getType()));
+  }
+
   // If all PHI operands are the same operation, pull them through the PHI,
   // reducing code size.
   if (isa<Instruction>(PN.getIncomingValue(0)) &&


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D87965.292966.patch
Type: text/x-patch
Size: 2824 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200919/b0371002/attachment.bin>