Implement a FIXME for better poison handling in SimplifyCFG.

[nikic]

Test?

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

335

You need to check PoisonValue first, because PoisonValue is also an UndefValue.

341

The !isa<PoisonValue> checks shouldn't be necessary.

[resistor]

I don't have a test that exercises this, just following up on the FIXME comments.

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

341

Why?

[resistor]

Update for comments

[nikic]

This is still missing tests.

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

335

What I meant here is to move the PoisonValue checks below to above the UndefValue checks, so they are handled first.

341

If one value is poison, the other doesn't matter. Now, if both were actually poison this would already be handled by IV0 == IV1 above, but there's no need to spell it out here.

[resistor]

Update for feedback

[resistor]

I have not been able to construct a test for this manually, and didn't find any hits for it in the test-suite.

llvm/lib/Transforms/Utils/SimplifyCFG.cpp

 /// is provided, and *both* of the values are present in the set,
 /// then they are considered equal.
 static bool IncomingValuesAreCompatible(
     BasicBlock *BB, ArrayRef<BasicBlock *> IncomingBlocks,
     SmallPtrSetImpl<Value *> *EquivalenceSet = nullptr) {
   assert(IncomingBlocks.size() == 2 &&
          "Only for a pair of incoming blocks at the time!");
 
-  // FIXME: it is okay if one of the incoming values is an `undef` value,
-  //        iff the other incoming value is guaranteed to be a non-poison value.
-  // FIXME: it is okay if one of the incoming values is a `poison` value.
   return all_of(BB->phis(), [IncomingBlocks, EquivalenceSet](PHINode &PN) {
     Value *IV0 = PN.getIncomingValueForBlock(IncomingBlocks[0]);
     Value *IV1 = PN.getIncomingValueForBlock(IncomingBlocks[1]);
     if (IV0 == IV1)
       return true;
     if (EquivalenceSet && EquivalenceSet->contains(IV0) &&
         EquivalenceSet->contains(IV1))
       return true;
+
+    // It is okay if one of the incoming values is an `undef` value, iff the
+    // other incoming value is guaranteed to be a non-poison value.
+    if (isa<UndefValue>(IV0) && isGuaranteedNotToBePoison(IV1))
+      return true;
+    if (isa<UndefValue>(IV1) && isGuaranteedNotToBePoison(IV0))
+      return true;
+
+    // It is okay if one of the incoming values is a `poison` value.
+    if (isa<PoisonValue>(IV0) && !isa<PoisonValue>(IV1))
+      return true;
+    if (isa<PoisonValue>(IV1) && !isa<PoisonValue>(IV0))
+      return true;
     return false;
   });
 }
 
 /// Return true if it is safe to merge these two
 /// terminator instructions together.
 static bool
 SafeToMergeTerminators(Instruction *SI1, Instruction *SI2,