[PATCH] D97495: [SimplifyCFG] avoid illegal phi with both poison and undef

Sanjay Patel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Feb 25 13:25:34 PST 2021


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

In the example based on:
https://bugs.llvm.org/PR49218
...we are crashing because poison is a subclass of undef, so we merge blocks and create:

  PHI node has multiple entries for the same basic block with different incoming values!
    %k3 = phi i64 [ poison, %entry ], [ %k3, %g ], [ undef, %entry ]

I think we could soften the poison to undef, but it seems unlikely that this would matter in any real code.


https://reviews.llvm.org/D97495

Files:
  llvm/lib/Transforms/Utils/Local.cpp
  llvm/test/Transforms/SimplifyCFG/poison-merge.ll


Index: llvm/test/Transforms/SimplifyCFG/poison-merge.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/SimplifyCFG/poison-merge.ll
@@ -0,0 +1,38 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -simplifycfg -simplifycfg-require-and-preserve-domtree=1 -keep-loops=0 < %s | FileCheck %s
+
+; Do not crash trying to merge poison and undef into a single phi.
+
+define i32 @PR49218(i32 %x) {
+; CHECK-LABEL: @PR49218(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    switch i32 [[X:%.*]], label [[EXIT:%.*]] [
+; CHECK-NEXT:    i32 4, label [[LOOP:%.*]]
+; CHECK-NEXT:    i32 12, label [[G:%.*]]
+; CHECK-NEXT:    ]
+; CHECK:       loop:
+; CHECK-NEXT:    [[K2:%.*]] = phi i64 [ [[K3:%.*]], [[G]] ], [ undef, [[ENTRY:%.*]] ]
+; CHECK-NEXT:    br label [[G]]
+; CHECK:       g:
+; CHECK-NEXT:    [[K3]] = phi i64 [ [[K2]], [[LOOP]] ], [ poison, [[ENTRY]] ]
+; CHECK-NEXT:    br label [[LOOP]]
+; CHECK:       exit:
+; CHECK-NEXT:    ret i32 undef
+;
+entry:
+  switch i32 %x, label %exit [
+  i32 4, label %loop
+  i32 12, label %g
+  ]
+
+loop:
+  %k2 = phi i64 [ %k3, %g ], [ undef, %entry ]
+  br label %g
+
+g:
+  %k3 = phi i64 [ %k2, %loop ], [ poison, %entry ]
+  br label %loop
+
+exit:
+  ret i32 undef
+}
Index: llvm/lib/Transforms/Utils/Local.cpp
===================================================================
--- llvm/lib/Transforms/Utils/Local.cpp
+++ llvm/lib/Transforms/Utils/Local.cpp
@@ -795,8 +795,10 @@
 
 /// Return true if we can choose one of these values to use in place of the
 /// other. Note that we will always choose the non-undef value to keep.
-static bool CanMergeValues(Value *First, Value *Second) {
-  return First == Second || isa<UndefValue>(First) || isa<UndefValue>(Second);
+/// The XOR logic prevents merging poison with undef. We can merge either of
+/// those into a defined value, but not into each other.
+static bool canMergeValues(Value *First, Value *Second) {
+  return First == Second || (isa<UndefValue>(First) ^ isa<UndefValue>(Second));
 }
 
 /// Return true if we can fold BB, an almost-empty BB ending in an unconditional
@@ -828,7 +830,7 @@
       for (unsigned PI = 0, PE = PN->getNumIncomingValues(); PI != PE; ++PI) {
         BasicBlock *IBB = PN->getIncomingBlock(PI);
         if (BBPreds.count(IBB) &&
-            !CanMergeValues(BBPN->getIncomingValueForBlock(IBB),
+            !canMergeValues(BBPN->getIncomingValueForBlock(IBB),
                             PN->getIncomingValue(PI))) {
           LLVM_DEBUG(dbgs()
                      << "Can't fold, phi node " << PN->getName() << " in "
@@ -846,7 +848,7 @@
         // of the block.
         BasicBlock *IBB = PN->getIncomingBlock(PI);
         if (BBPreds.count(IBB) &&
-            !CanMergeValues(Val, PN->getIncomingValue(PI))) {
+            !canMergeValues(Val, PN->getIncomingValue(PI))) {
           LLVM_DEBUG(dbgs() << "Can't fold, phi node " << PN->getName()
                             << " in " << Succ->getName()
                             << " is conflicting with regard to common "


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D97495.326471.patch
Type: text/x-patch
Size: 3118 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210225/c829c2fd/attachment.bin>


More information about the llvm-commits mailing list