[ConstraintElim] Check of other OP is guaranteed to not be undef/poison.

[fhahn]

Address the TODO from #75750 by checking if the second operand may be poison. We can simplify the first operand if it is implied by the other operand, if the other operand is guaranteed to not be undef/poison. If it may be poison, it would mean the select would be unconditionally poison after the transform.

Note that the original TODO incorrectly said that the first operand would need checking, but it is the other one that needs checking.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Florian Hahn (fhahn)

Changes

Address the TODO from #75750 by checking if the second operand may be poison. We can simplify the first operand if it is implied by the other operand, if the other operand is guaranteed to not be undef/poison. If it may be poison, it would mean the select would be unconditionally poison after the transform.

Note that the original TODO incorrectly said that the first operand would need checking, but it is the other one that needs checking.

---

Full diff: https://github.com/llvm/llvm-project/pull/76182.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/Scalar/ConstraintElimination.cpp (+5-4)
• (modified) llvm/test/Transforms/ConstraintElimination/and-implied-by-operands.ll (+1-1)

diff --git a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
index 98cfadddee8efb..1780e23af081f2 100644
--- a/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
+++ b/llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
@@ -1378,10 +1378,11 @@ static bool checkOrAndOpImpliedByOther(
   CmpInst *CmpToCheck = cast<CmpInst>(CB.getInstructionToSimplify());
   unsigned OtherOpIdx = JoinOp->getOperand(0) == CmpToCheck ? 1 : 0;
 
-  // Don't try to simplify the first condition of a select by the second, as
-  // this may make the select more poisonous than the original one.
-  // TODO: check if the first operand may be poison.
-  if (OtherOpIdx != 0 && isa<SelectInst>(JoinOp))
+  // Don't try to simplify the first condition of a select using the other
+  // condition, if this makes the select more poisonous than the original one.
+  if (OtherOpIdx != 0 && isa<SelectInst>(JoinOp) &&
+      !isGuaranteedNotToBeUndefOrPoison(JoinOp->getOperand(OtherOpIdx), nullptr,
+                                        JoinOp))
     return false;
 
   if (!match(JoinOp->getOperand(OtherOpIdx),
diff --git a/llvm/test/Transforms/ConstraintElimination/and-implied-by-operands.ll b/llvm/test/Transforms/ConstraintElimination/and-implied-by-operands.ll
index 5c49ca0e96f309..329bd94beac852 100644
--- a/llvm/test/Transforms/ConstraintElimination/and-implied-by-operands.ll
+++ b/llvm/test/Transforms/ConstraintElimination/and-implied-by-operands.ll
@@ -486,7 +486,7 @@ define i1 @and_select_second_implies_first_guaranteed_not_poison(ptr noundef %A,
 ; CHECK-NEXT:    [[GEP:%.*]] = getelementptr inbounds ptr, ptr [[B]], i64 -1
 ; CHECK-NEXT:    [[C_2:%.*]] = icmp ugt ptr [[GEP]], [[A]]
 ; CHECK-NEXT:    call void @no_noundef(i1 [[C_2]])
-; CHECK-NEXT:    [[AND:%.*]] = select i1 [[C_1]], i1 [[C_2]], i1 false
+; CHECK-NEXT:    [[AND:%.*]] = select i1 true, i1 [[C_2]], i1 false
 ; CHECK-NEXT:    ret i1 [[AND]]
 ;
 entry:

[nikic]

If the operand isn't poison, wouldn't the logical op get converted to a bitwise op by InstCombine? Are we concerned about phase ordering issues here?

[fhahn]

If the operand isn't poison, wouldn't the logical op get converted to a bitwise op by InstCombine? Are we concerned about phase ordering issues here?

Let me check if there are any cases where this is actually needed; it looks like SimplifyCFG also already tries to introduce bitwise ops directly, so together with instcombine most (probably all) cases should already covered.

[dtcxzyw]

It seems like this patch doesn't change anything :(

[fhahn]

Yeah, I checked and it looks like in all cases selects are already transformed to bitwise ops in practice, so this isn't needed.

I am wondering if it would be worth in general to verify some canonical form assumptions at certain points, to catch cases where canonicalization where missed.