[ValueTracking][InstCombine] Add a new API to allow to ignore poison generating flags or metadatas when implying poison

[StephenFan]

Summary

This patch add a new API impliesPoisonIgnoreFlagsOrMetadatas which is
the same as impliesPoison but ignoring poison generating flags or
metadatas in the process of implying poison and recording these ignored
instructions.

In InstCombineSelect, replacing impliesPoison with
impliesPoisonIgnoreFlagsOrMetadatas to allow more patterns like
select i1 %a, i1 %b, i1 false to be optimized to and/or instructions
by droping the poison generating flags or metadatas.

[StephenFan]

llvm/test/Transforms/InstCombine/ispow2.ll

917–921

I think this regression can be fixed If InstCombine matches this pattern.

[goldstein.w.n]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

2922

Imo this should be split into a sepera

llvm/test/Transforms/InstCombine/ispow2.ll

917–921

What changed in the impl that caused this?

[goldstein.w.n]

llvm/lib/Analysis/ValueTracking.cpp

6771

true?

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

2942

Will you ever not loop through instruction and do dropPoisonGenerating...? Maybe it should done by impliesPoisonIgnore... and if its not always with a bool to control it.

[nikic]

llvm/lib/Analysis/ValueTracking.cpp

6748

Hm, maybe just directly expose the API with the optional arg? Not sure we need one without the arg and one with it required... No strong opinion though.

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

2942

If it's part of a larger transform, one may only want to drop the flags after additional conditions have been checked as well. Analysis utils generally aren't allowed to modify instructions.

2947

Why did this code get moved?

llvm/test/Transforms/InstCombine/ispow2.ll

917–921

I expect this got handled by the select value equivalence fold before. It's okay to regress for now because it was enabled as a side-effect of https://github.com/llvm/llvm-project/commit/bf23b4031eeabfccd46a25ce68414d45ae761304, and this patch basically undoes the effects of that one on this file.

[goldstein.w.n]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

2922

err, ignore this. started writing it but forgot to delete.

[goldstein.w.n]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

2942

then maybe an API in instcombine, or at least a static helper/lambda in the file.

[StephenFan]

Move back codes that optimize select

[goldstein.w.n]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

2936

Still in favor of putting this loop in some API function. It can be in InstCombine.

[nikic]

llvm/test/Transforms/InstCombine/and-fcmp.ll

393 ↗

(On Diff #519871)

Okay, now I understand why you moved the foldLogicOfFCmps() call. Please feel free to move it back, just add a comment to explain why we do it first.

[StephenFan]

Use lambda to drop posion flags or metadata and move back codes.

[StephenFan]

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

2942

I am sorry that I ignored your comment.

[nikic]

LGTM

llvm/include/llvm/Analysis/ValueTracking.h

934

metadatas -> metadata

935

instructions ignored -> ignored instructions

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

2926

SmallVectorImpl<Instruction *> & -> ArrayRef<Instruction *>

[StephenFan]

update comments and change to arrayref

[fhahn]

I am seeing regressions due to dropping poison generating flags. For the test case below, we now drop the inbounds from %gep to select with and but it is not clear to me that replacing select with and outweighs the loss of poison generating flags?

The particular case I am looking at requires inbounds on the GEP to trigger further optimizations.

define i1 @test(ptr %base, i64 noundef %len, ptr %p2) {
bb:
  %gep = getelementptr inbounds i32, ptr %base, i64 %len
  %c.1 = icmp uge ptr %p2, %base
  %c.2 = icmp ult ptr %p2, %gep
  %select = select i1 %c.1, i1 %c.2, i1 false
  ret i1 %select
}

[fhahn]

I added a phase-ordering test that shows a slight regression in 7c91d82ab912fae8bafc1137d4c9b17bcfb7eba7. It would be great if you could take a look. Perhaps this shouldn't be applied if poison-generating flags need to be dropped. Or only be dropped late in the pipeline.

[nikic]

I would be okay with reverting the change. I don't think we had particularly strong motivation for it (iirc it was basically just that some selects were not converted, without actually blocking any following optimizations), so if it causes regressions in practice, it's probably best to not do it. Our optimizations for logical and/or are pretty good.

[StephenFan]

In D149404#4376971, @nikic wrote:

I would be okay with reverting the change. I don't think we had particularly strong motivation for it (iirc it was basically just that some selects were not converted, without actually blocking any following optimizations), so if it causes regressions in practice, it's probably best to not do it. Our optimizations for logical and/or are pretty good.

What about just handling poison generating metadata?

[fhahn]

In D149404#4377682, @StephenFan wrote:

In D149404#4376971, @nikic wrote:

I would be okay with reverting the change. I don't think we had particularly strong motivation for it (iirc it was basically just that some selects were not converted, without actually blocking any following optimizations), so if it causes regressions in practice, it's probably best to not do it. Our optimizations for logical and/or are pretty good.

What about just handling poison generating metadata?

I went ahead and reverted the change for now. I am not sure, but I would assume dropping poison generating metadata could lead to similarly missed optimizations unfortunately.

llvm/include/llvm/Analysis/ValueTracking.h

bool canCreateUndefOrPoison(const Operator *Op,
                             bool ConsiderFlagsAndMetadata = true);
 bool canCreatePoison(const Operator *Op, bool ConsiderFlagsAndMetadata = true);
 
 /// Return true if V is poison given that ValAssumedPoison is already poison.
 /// For example, if ValAssumedPoison is `icmp X, 10` and V is `icmp X, 5`,
 /// impliesPoison returns true.
 bool impliesPoison(const Value *ValAssumedPoison, const Value *V);
 
+/// Return true if V is poison given that ValAssumedPoison is already poison.
+/// Poison generating flags or metadata are ignored in the process of implying.
+/// And the ignored instructions will be recorded in IgnoredInsts.
+bool impliesPoisonIgnoreFlagsOrMetadata(
+    Value *ValAssumedPoison, const Value *V,
+    SmallVectorImpl<Instruction *> &IgnoredInsts);
+
 /// Return true if this function can prove that V does not have undef bits
 /// and is never poison. If V is an aggregate value or vector, check whether
 /// all elements (except padding) are not undef or poison.
 /// Note that this is different from canCreateUndefOrPoison because the
 /// function assumes Op's operands are not poison/undef.
 ///
 /// If CtxI and DT are specified this method performs flow-sensitive analysis
 /// and returns true if it is guaranteed to be never undef or poison

llvm/lib/Analysis/ValueTracking.cpp

if (const auto *I = dyn_cast<Instruction>(V)) {
     if (match(I, m_ExtractValue(m_WithOverflowInst(II))) &&
         (match(ValAssumedPoison, m_ExtractValue(m_Specific(II))) ||
          llvm::is_contained(II->args(), ValAssumedPoison)))
       return true;
   }
   return false;
 }
 
-static bool impliesPoison(const Value *ValAssumedPoison, const Value *V,
-                          unsigned Depth) {
+static bool
+impliesPoison(Value *ValAssumedPoison, const Value *V, unsigned Depth,
+              SmallVectorImpl<Instruction *> *IgnoredInsts = nullptr) {
   if (isGuaranteedNotToBePoison(ValAssumedPoison))
     return true;
 
   if (directlyImpliesPoison(ValAssumedPoison, V, /* Depth */ 0))
     return true;
 
   const unsigned MaxDepth = 2;
   if (Depth >= MaxDepth)
     return false;
 
-  const auto *I = dyn_cast<Instruction>(ValAssumedPoison);
-  if (I && !canCreatePoison(cast<Operator>(I))) {
-    return all_of(I->operands(), [=](const Value *Op) {
-      return impliesPoison(Op, V, Depth + 1);
-    });
-  }
+  auto *I = dyn_cast<Instruction>(ValAssumedPoison);
+  if (!I || canCreatePoison(cast<Operator>(I),
+                            /*ConsiderFlagsAndMetadata*/ !IgnoredInsts))
+    return false;
+
+  for (Value *Op : I->operands())
+    if (!impliesPoison(Op, V, Depth + 1, IgnoredInsts))
       return false;
+
+  if (IgnoredInsts && I->hasPoisonGeneratingFlagsOrMetadata())
+    IgnoredInsts->push_back(I);
+
+  return true;
 }
 
 bool llvm::impliesPoison(const Value *ValAssumedPoison, const Value *V) {
-  return ::impliesPoison(ValAssumedPoison, V, /* Depth */ 0);
+  return ::impliesPoison(const_cast<Value *>(ValAssumedPoison), V,
+                         /* Depth */ 0);
+}
+
+bool llvm::impliesPoisonIgnoreFlagsOrMetadata(
+    Value *ValAssumedPoison, const Value *V,
+    SmallVectorImpl<Instruction *> &IgnoredInsts) {
+  return ::impliesPoison(ValAssumedPoison, V, /* Depth */ 0, &IgnoredInsts);
 }
 
 static bool programUndefinedIfUndefOrPoison(const Value *V,
                                             bool PoisonOnly);
 
 static bool isGuaranteedNotToBeUndefOrPoison(const Value *V,
                                              AssumptionCache *AC,
                                              const Instruction *CtxI,

llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp

Instruction *InstCombinerImpl::foldSelectOfBools(SelectInst &SI) {
 
   // Avoid potential infinite loops by checking for non-constant condition.
   // TODO: Can we assert instead by improving canonicalizeSelectToShuffle()?
   //       Scalar select must have simplified?
   if (!SelType->isIntOrIntVectorTy(1) || isa<Constant>(CondVal) ||
       TrueVal->getType() != CondVal->getType())
     return nullptr;
 
   auto *One = ConstantInt::getTrue(SelType);
   auto *Zero = ConstantInt::getFalse(SelType);
   Value *A, *B, *C, *D;
 
+  auto dropPoisonGeneratingFlagsAndMetadata =
+      [](ArrayRef<Instruction *> Insts) {
+        for (auto *I : Insts)
+          I->dropPoisonGeneratingFlagsAndMetadata();
+      };
   // Folding select to and/or i1 isn't poison safe in general. impliesPoison
   // checks whether folding it does not convert a well-defined value into
   // poison.
   if (match(TrueVal, m_One())) {
-    if (impliesPoison(FalseVal, CondVal)) {
-      // Change: A = select B, true, C --> A = or B, C
-      return BinaryOperator::CreateOr(CondVal, FalseVal);
-    }
-
     if (auto *LHS = dyn_cast<FCmpInst>(CondVal))
       if (auto *RHS = dyn_cast<FCmpInst>(FalseVal))
         if (Value *V = foldLogicOfFCmps(LHS, RHS, /*IsAnd*/ false,
                                         /*IsSelectLogical*/ true))
           return replaceInstUsesWith(SI, V);
 
+    // Some patterns can be matched by both of the above and following
+    // combinations. Because we need to drop poison generating
+    // flags and metadatas for the following combination, it has less priority
+    // than the above combination.
+    SmallVector<Instruction *> IgnoredInsts;
+    if (impliesPoisonIgnoreFlagsOrMetadata(FalseVal, CondVal, IgnoredInsts)) {
+      dropPoisonGeneratingFlagsAndMetadata(IgnoredInsts);
+      // Change: A = select B, true, C --> A = or B, C
+      return BinaryOperator::CreateOr(CondVal, FalseVal);
+    }
+
     // (A && B) || (C && B) --> (A || C) && B
     if (match(CondVal, m_LogicalAnd(m_Value(A), m_Value(B))) &&
         match(FalseVal, m_LogicalAnd(m_Value(C), m_Value(D))) &&
         (CondVal->hasOneUse() || FalseVal->hasOneUse())) {
       bool CondLogicAnd = isa<SelectInst>(CondVal);
       bool FalseLogicAnd = isa<SelectInst>(FalseVal);
       auto AndFactorization = [&](Value *Common, Value *InnerCond,
                                   Value *InnerVal,
if (match(CondVal, m_LogicalAnd(m_Value(A), m_Value(B))) &&
       if (B == C)
         return AndFactorization(B, A, D);
       if (B == D)
         return AndFactorization(B, A, C, CondLogicAnd && FalseLogicAnd);
     }
   }
 
   if (match(FalseVal, m_Zero())) {
-    if (impliesPoison(TrueVal, CondVal)) {
-      // Change: A = select B, C, false --> A = and B, C
-      return BinaryOperator::CreateAnd(CondVal, TrueVal);
-    }
-
     if (auto *LHS = dyn_cast<FCmpInst>(CondVal))
       if (auto *RHS = dyn_cast<FCmpInst>(TrueVal))
         if (Value *V = foldLogicOfFCmps(LHS, RHS, /*IsAnd*/ true,
                                         /*IsSelectLogical*/ true))
           return replaceInstUsesWith(SI, V);
 
+    // Some patterns can be matched by both of the above and following
+    // combinations. Because we need to drop poison generating
+    // flags and metadatas for the following combination, it has less priority
+    // than the above combination.
+    SmallVector<Instruction *> IgnoredInsts;
+    if (impliesPoisonIgnoreFlagsOrMetadata(TrueVal, CondVal, IgnoredInsts)) {
+      dropPoisonGeneratingFlagsAndMetadata(IgnoredInsts);
+      // Change: A = select B, C, false --> A = and B, C
+      return BinaryOperator::CreateAnd(CondVal, TrueVal);
+    }
+
     // (A || B) && (C || B) --> (A && C) || B
     if (match(CondVal, m_LogicalOr(m_Value(A), m_Value(B))) &&
         match(TrueVal, m_LogicalOr(m_Value(C), m_Value(D))) &&
         (CondVal->hasOneUse() || TrueVal->hasOneUse())) {
       bool CondLogicOr = isa<SelectInst>(CondVal);
       bool TrueLogicOr = isa<SelectInst>(TrueVal);
       auto OrFactorization = [&](Value *Common, Value *InnerCond,
                                  Value *InnerVal,

llvm/test/Transforms/InstCombine/ispow2.ll

;
   ret i1 %r
 }
 
 define i1 @is_pow2_ctpop_wrong_cmp_op1_logical(i32 %x) {
 ; CHECK-LABEL: @is_pow2_ctpop_wrong_cmp_op1_logical(
 ; CHECK-NEXT:    [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i32 [[T0]], 3
 ; CHECK-NEXT:    [[NOTZERO:%.*]] = icmp ne i32 [[X]], 0
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[NOTZERO]], i1 [[CMP]], i1 false
+; CHECK-NEXT:    [[R:%.*]] = and i1 [[NOTZERO]], [[CMP]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp ult i32 %t0, 3
   %notzero = icmp ne i32 %x, 0
   %r = select i1 %notzero, i1 %cmp, i1 false
   ret i1 %r
 }
;
   ret i1 %r
 }
 
 define i1 @is_pow2_ctpop_wrong_cmp_op2_logical(i32 %x) {
 ; CHECK-LABEL: @is_pow2_ctpop_wrong_cmp_op2_logical(
 ; CHECK-NEXT:    [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i32 [[T0]], 2
 ; CHECK-NEXT:    [[NOTZERO:%.*]] = icmp ne i32 [[X]], 1
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[NOTZERO]], i1 [[CMP]], i1 false
+; CHECK-NEXT:    [[R:%.*]] = and i1 [[NOTZERO]], [[CMP]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp ult i32 %t0, 2
   %notzero = icmp ne i32 %x, 1
   %r = select i1 %notzero, i1 %cmp, i1 false
   ret i1 %r
 }
;
   ret i1 %r
 }
 
 define i1 @is_pow2_ctpop_wrong_pred1_logical(i32 %x) {
 ; CHECK-LABEL: @is_pow2_ctpop_wrong_pred1_logical(
 ; CHECK-NEXT:    [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i32 [[T0]], 2
 ; CHECK-NEXT:    [[NOTZERO:%.*]] = icmp ne i32 [[X]], 0
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[NOTZERO]], i1 [[CMP]], i1 false
+; CHECK-NEXT:    [[R:%.*]] = and i1 [[NOTZERO]], [[CMP]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp ugt i32 %t0, 2
   %notzero = icmp ne i32 %x, 0
   %r = select i1 %notzero, i1 %cmp, i1 false
   ret i1 %r
 }
;
   ret i1 %r
 }
 
 define i1 @is_pow2_ctpop_wrong_pred2_logical(i32 %x) {
 ; CHECK-LABEL: @is_pow2_ctpop_wrong_pred2_logical(
 ; CHECK-NEXT:    [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ult i32 [[T0]], 2
 ; CHECK-NEXT:    [[CMP2:%.*]] = icmp sgt i32 [[X]], 0
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[CMP2]], i1 [[CMP]], i1 false
+; CHECK-NEXT:    [[R:%.*]] = and i1 [[CMP2]], [[CMP]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp ult i32 %t0, 2
   %cmp2 = icmp sgt i32 %x, 0
   %r = select i1 %cmp2, i1 %cmp, i1 false
   ret i1 %r
 }
;
   ret i1 %r
 }
 
 define i1 @isnot_pow2_ctpop_wrong_cmp_op1_logical(i32 %x) {
 ; CHECK-LABEL: @isnot_pow2_ctpop_wrong_cmp_op1_logical(
 ; CHECK-NEXT:    [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i32 [[T0]], 2
 ; CHECK-NEXT:    [[ISZERO:%.*]] = icmp eq i32 [[X]], 0
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[ISZERO]], i1 true, i1 [[CMP]]
+; CHECK-NEXT:    [[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp ugt i32 %t0, 2
   %iszero = icmp eq i32 %x, 0
   %r = select i1 %iszero, i1 true, i1 %cmp
   ret i1 %r
 }
;
   ret i1 %r
 }
 
 define i1 @isnot_pow2_ctpop_wrong_cmp_op2_logical(i32 %x) {
 ; CHECK-LABEL: @isnot_pow2_ctpop_wrong_cmp_op2_logical(
 ; CHECK-NEXT:    [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i32 [[T0]], 1
 ; CHECK-NEXT:    [[ISZERO:%.*]] = icmp eq i32 [[X]], 1
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[ISZERO]], i1 true, i1 [[CMP]]
+; CHECK-NEXT:    [[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp ugt i32 %t0, 1
   %iszero = icmp eq i32 %x, 1
   %r = select i1 %iszero, i1 true, i1 %cmp
   ret i1 %r
 }
;
   ret i1 %r
 }
 
 define i1 @isnot_pow2_ctpop_wrong_pred2_logical(i32 %x) {
 ; CHECK-LABEL: @isnot_pow2_ctpop_wrong_pred2_logical(
 ; CHECK-NEXT:    [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ugt i32 [[T0]], 1
 ; CHECK-NEXT:    [[CMP2:%.*]] = icmp slt i32 [[X]], 0
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[CMP2]], i1 true, i1 [[CMP]]
+; CHECK-NEXT:    [[R:%.*]] = or i1 [[CMP2]], [[CMP]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp ugt i32 %t0, 1
   %cmp2 = icmp slt i32 %x, 0
   %r = select i1 %cmp2, i1 true, i1 %cmp
   ret i1 %r
 }
;
   ret i1 %r
 }
 
 define i1 @is_pow2or0_ctpop_wrong_cmp_op1_logical(i32 %x) {
 ; CHECK-LABEL: @is_pow2or0_ctpop_wrong_cmp_op1_logical(
 ; CHECK-NEXT:    [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[T0]], 3
 ; CHECK-NEXT:    [[ISZERO:%.*]] = icmp eq i32 [[X]], 0
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[ISZERO]], i1 true, i1 [[CMP]]
+; CHECK-NEXT:    [[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp eq i32 %t0, 3
   %iszero = icmp eq i32 %x, 0
   %r = select i1 %iszero, i1 true, i1 %cmp
   ret i1 %r
 }
;
   %cmp = icmp ne i32 %t0, 1
   %iszero = icmp ne i32 %x, 0
   %r = or i1 %iszero, %cmp
   ret i1 %r
 }
 
 define i1 @is_pow2or0_ctpop_wrong_pred2_logical(i32 %x) {
 ; CHECK-LABEL: @is_pow2or0_ctpop_wrong_pred2_logical(
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ne i32 [[T0]], 1
+; CHECK-NEXT:    [[ISZERO:%.*]] = icmp ne i32 [[X]], 0
+; CHECK-NEXT:    [[R:%.*]] = or i1 [[ISZERO]], [[CMP]]
+; CHECK-NEXT:    ret i1 [[R]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp ne i32 %t0, 1
   %iszero = icmp ne i32 %x, 0
   %r = select i1 %iszero, i1 true, i1 %cmp
   ret i1 %r
 }
 
;
   ret i1 %r
 }
 
 define i1 @isnot_pow2nor0_ctpop_wrong_cmp_op1_logical(i32 %x) {
 ; CHECK-LABEL: @isnot_pow2nor0_ctpop_wrong_cmp_op1_logical(
 ; CHECK-NEXT:    [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ne i32 [[T0]], 5
 ; CHECK-NEXT:    [[NOTZERO:%.*]] = icmp ne i32 [[X]], 0
-; CHECK-NEXT:    [[R:%.*]] = select i1 [[NOTZERO]], i1 [[CMP]], i1 false
+; CHECK-NEXT:    [[R:%.*]] = and i1 [[NOTZERO]], [[CMP]]
 ; CHECK-NEXT:    ret i1 [[R]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp ne i32 %t0, 5
   %notzero = icmp ne i32 %x, 0
   %r = select i1 %notzero, i1 %cmp, i1 false
   ret i1 %r
 }
;
   %cmp = icmp eq i32 %t0, 1
   %notzero = icmp eq i32 %x, 0
   %r = and i1 %notzero, %cmp
   ret i1 %r
 }
 
 define i1 @isnot_pow2nor0_ctpop_wrong_pred2_logical(i32 %x) {
 ; CHECK-LABEL: @isnot_pow2nor0_ctpop_wrong_pred2_logical(
-; CHECK-NEXT:    ret i1 false
+; CHECK-NEXT:    [[T0:%.*]] = tail call i32 @llvm.ctpop.i32(i32 [[X:%.*]]), !range [[RNG0]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp eq i32 [[T0]], 1
+; CHECK-NEXT:    [[NOTZERO:%.*]] = icmp eq i32 [[X]], 0
+; CHECK-NEXT:    [[R:%.*]] = and i1 [[NOTZERO]], [[CMP]]
+; CHECK-NEXT:    ret i1 [[R]]
 ;
   %t0 = tail call i32 @llvm.ctpop.i32(i32 %x)
   %cmp = icmp eq i32 %t0, 1
   %notzero = icmp eq i32 %x, 0
   %r = select i1 %notzero, i1 %cmp, i1 false
   ret i1 %r
 }
 

llvm/test/Transforms/InstCombine/prevent-cmp-merge.ll

;
   %cmp1 = icmp eq i32 %sub, 0
   %cmp2 = icmp eq i32 %sub, 31
   %sel = or i1 %cmp1, %cmp2
   ret i1 %sel
 }
 
 define zeroext i1 @test3_logical(i32 %lhs, i32 %rhs) {
 ; CHECK-LABEL: @test3_logical(
-; CHECK-NEXT:    [[SUB:%.*]] = sub nsw i32 [[LHS:%.*]], [[RHS:%.*]]
+; CHECK-NEXT:    [[SUB:%.*]] = sub i32 [[LHS:%.*]], [[RHS:%.*]]
 ; CHECK-NEXT:    [[CMP1:%.*]] = icmp eq i32 [[LHS]], [[RHS]]
 ; CHECK-NEXT:    [[CMP2:%.*]] = icmp eq i32 [[SUB]], 31
-; CHECK-NEXT:    [[SEL:%.*]] = select i1 [[CMP1]], i1 true, i1 [[CMP2]]
+; CHECK-NEXT:    [[SEL:%.*]] = or i1 [[CMP1]], [[CMP2]]
 ; CHECK-NEXT:    ret i1 [[SEL]]
 ;
 
   %sub = sub nsw i32 %lhs, %rhs
   %cmp1 = icmp eq i32 %sub, 0
   %cmp2 = icmp eq i32 %sub, 31
   %sel = select i1 %cmp1, i1 true, i1 %cmp2
   ret i1 %sel
 }