[ValueTracking] isKnownNonZero() should take non-null-ness assumptions into consideration (PR43267)

[lebedev.ri]

Summary

It is pretty common to assume that something is not zero.
Even optimizer itself sometimes emits such assumptions
(e.g. addAssumeNonNull() in PromoteMemoryToRegister.cpp).

But we currently don't deal with such assumptions :)
The only way isKnownNonZero() handles assumptions is
by calling computeKnownBits() which calls computeKnownBitsFromAssume().
But x != 0 does not tell us anything about set bits,
it only says that there are *some* set bits.
So naturally, KnownBits does not get populated,
and we fail to make use of this assumption.

I propose to deal with this special case by special-casing it
via adding a isKnownNonZeroFromAssume() that returns boolean
when there is such an assumption.

https://rise4fun.com/Alive/6yR

Fixes PR43267.

[nikic]

Have you considered integrating this in isKnownNonNullFromDominatingCondition() instead? It already iterates over all users of V != 0 style conditions and could easily consider assumes in addition to branches. That may be more efficient than handling it separately.

[nikic]

I should mention that CVP also handles this, but as most things in CVP it only works across basic blocks (until D69686 lands). I do think it's reasonable to also have this in ValueTracking though.

[lebedev.ri]

In D71660#1789726, @nikic wrote:

Have you considered integrating this in isKnownNonNullFromDominatingCondition() instead? It already iterates over all users of V != 0 style conditions and could easily consider assumes in addition to branches.
That may be more efficient than handling it separately.

I'm not sure why it would be better than relying on the infrastructure specifically designed to handling assumptions?
It isn't obvious to me which approach is more performant, without benchmarks either approach may be much worse than the other one.

[nikic]

In D71660#1789759, @lebedev.ri wrote:

In D71660#1789726, @nikic wrote:

Have you considered integrating this in isKnownNonNullFromDominatingCondition() instead? It already iterates over all users of V != 0 style conditions and could easily consider assumes in addition to branches.
That may be more efficient than handling it separately.

I'm not sure why it would be better than relying on the infrastructure specifically designed to handling assumptions?
It isn't obvious to me which approach is more performant, without benchmarks either approach may be much worse than the other one.

It's a question of which existing infrastructure you want to reuse ;) isKnownNonNullFromDominatingCondition() already handles many other scenarios involving dominating conditions and assume-like operations, but misses a check for the actual assumes.

In particular https://github.com/llvm-mirror/llvm/blob/2c4ca6832fa6b306ee6a7010bfb80a3f2596f824/lib/Analysis/ValueTracking.cpp#L1965-L1968 already deals with llvm.experimental.guard(). Adding support for assumes should be as simple as replacing isGuard() with isGuard() || isAssume() there.

Does that make sense?

[lebedev.ri]

In D71660#1789768, @nikic wrote:

In D71660#1789759, @lebedev.ri wrote:

In D71660#1789726, @nikic wrote:

Have you considered integrating this in isKnownNonNullFromDominatingCondition() instead? It already iterates over all users of V != 0 style conditions and could easily consider assumes in addition to branches.
That may be more efficient than handling it separately.

I'm not sure why it would be better than relying on the infrastructure specifically designed to handling assumptions?
It isn't obvious to me which approach is more performant, without benchmarks either approach may be much worse than the other one.

It's a question of which existing infrastructure you want to reuse ;) isKnownNonNullFromDominatingCondition() already handles many other scenarios involving dominating conditions and assume-like operations, but misses a check for the actual assumes.

In particular https://github.com/llvm-mirror/llvm/blob/2c4ca6832fa6b306ee6a7010bfb80a3f2596f824/lib/Analysis/ValueTracking.cpp#L1965-L1968 already deals with llvm.experimental.guard(). Adding support for assumes should be as simple as replacing isGuard() with isGuard() || isAssume() there.

Does that make sense?

I get that. The question is, given we have AssumptionCache abstraction, why do we want to avoid it.

[jdoerfert]

I think the code looks fine. (One minor comment inlined).

I will not accept because of the placement discussion. I have not strong opinion (yet) as long as we make sure not only LVI can use this.

llvm/lib/Analysis/ValueTracking.cpp

622

< is also fine, however unlikely.

[lebedev.ri]

Address inline comment - also handle strict signed comparisons - https://rise4fun.com/Alive/plslNp

[lebedev.ri]

llvm/lib/Analysis/ValueTracking.cpp

622

Right, https://rise4fun.com/Alive/plslNp

[nikic]

I get that. The question is, given we have AssumptionCache abstraction, why do we want to avoid it.

As this code now handles more predicates than the other code-path, I'm okay with your approach.

llvm/lib/Analysis/ValueTracking.cpp

622

As you went through the effort of supporting different predicates, possibly extend this to also handle non-zero constant? That is, not just x > 0 but also x > 100. Something along the lines of...

if (!match(Cmp, m_ICmp(Pred, m_V, m_APInt(C))))
  return false;

// Possibly without the non-canonical predicates?
switch (Pred) {
case ICmpInst::ICMP_NE:
  return C->isNullValue();
case ICmpInst::ICMP_UGT:
  return true;
case ICmpInst::ICMP_UGE:
  return !C->isNullValue();
case ICmpInst::ICMP_SGT:
  return C->isNonNegative();
case ICmpInst::ICMP_SGE:
  return C->isStrictlyPositive();
case ICmpInst::ICMP_SLT:
  return !C->isStrictlyPositive();
case ICmpInst::ICMP_SLE:
  return C->isNegative();
}

[xbolva00]

llvm/lib/Analysis/ValueTracking.cpp

592

getFunction()?

622

Tests?

[lebedev.ri]

Add FIXME comment, drop untested cases.

llvm/lib/Analysis/ValueTracking.cpp

622

On a second thought, i don't really feel like trying to handle everything at once.
X != 0 is the main motivation here.
Many other cases can be handled (https://rise4fun.com/Alive/6yR),
but i don't know that we need to special-handle them.
Do we know generic KnownBits (computeKnownBitsFromAssume)
does not handle them? I don't, and i won't until there are tests.

[lebedev.ri]

Though after sleeping on it, and finding the right microscope for this nail, other predicates added.
Hopefully the change can now proceed.

[lebedev.ri]

While there, handle assume(y u< v) too.

[jdoerfert]

LGTM. One suggestion below.

FWIW. The way I read the comments by @nikic, I think this can be pushed.

llvm/lib/Analysis/ValueTracking.cpp

647

I'd move the isValidAssumeForContext(I, Q.CxtI, Q.DT) as early as possible, assuming I haven't overlooked a case where it is not called.

[nikic]

llvm/lib/Analysis/ValueTracking.cpp

647

As isValidAssumeForContext() may need to perform a full block scan, I think it's preferable to check this condition last, and existing code seems to follow this, even if it means duplicating many isValidAssumeForContext() calls. I do agree though that it would be good to restructure the code in a way that only requires writing out the check once. Basically return icmpImpliesNonZero() && isValidAssumeForContext().

[nikic]

llvm/test/Transforms/InstCombine/assume.ll

300

Is this transform correct? As escape is not willreturn we don't necessarily reach the assume below, or?

[nikic]

llvm/test/Transforms/InstCombine/assume.ll

300

I think this might be a bug in isValidAssumeForContext(): https://github.com/llvm-mirror/llvm/blob/2c4ca6832fa6b306ee6a7010bfb80a3f2596f824/lib/Analysis/ValueTracking.cpp#L570 shouldn't be using std::next().

Probably this never mattered before, but I believe this needs to be fixed before this patch can land.

[lebedev.ri]

Addressed inline comments.

[nikic]

llvm/lib/Analysis/ValueTracking.cpp

571

Does just BasicBlock::const_iterator I(CxtI), IE(Inv); work?

610

Any particular reason to use m_ConstantInt() over m_APInt() here? The difference in vector handling?

635

nit: Unnecessary newline.

llvm/test/Transforms/InstCombine/assume.ll

273

I'm assuming that we first evaluate the icmp below, then the load is one-use and gets sunk. I'm wondering why the assume is not converted into !nonnull metadata after that, as the control dependence is now gone.

[lebedev.ri]

Some more nits addressed (these are several commits locally)

llvm/lib/Analysis/ValueTracking.cpp

610

I've never ever seen assumptions for vectors, so either should be fine.

llvm/test/Transforms/InstCombine/assume.ll

273

The best i can tell is because isEphemeralValueOf() says so?
I'm not really familiar with that, best to file a bug.

[nikic]

LGTM, thanks!

llvm/test/Transforms/InstCombine/assume.ll

273

Oooh, that makes sense... If we convert the assume to !nonnull, we'd just end up eliminating the load completely afterwards and lose the information.

It might make sense to slightly tweak the test (maybe add a use of %load in not_taken?) so the sinking does not happen. As it is right now, the test no longer tests what it was originally testing, even if the transform still fails for an unrelated reason.

[lebedev.ri]

Thank you for the review.

llvm/test/Transforms/InstCombine/assume.ll

273

The confusing thing is, if i manually sink load+cmp, then we add !nonnull and remove everything here.

[nikic]

llvm/test/Transforms/InstCombine/assume.ll

273

If the instruction is manually sunk, then we (probably) first convert the first assume into !nonnull (because there is still the other use in the second icmp, so not ephemeral), then fold the icmp, then dce the load. Not ideal, but probably also not really preventable.

llvm/lib/Analysis/ValueTracking.cpp

bool llvm::isValidAssumeForContext(const Instruction *Inv,
   // Don't let an assume affect itself - this would cause the problems
   // `isEphemeralValueOf` is trying to prevent, and it would also make
   // the loop below go out of bounds.
   if (Inv == CxtI)
     return false;
 
   // The context comes first, but they're both in the same block.
   // Make sure there is nothing in between that might interrupt
   // the control flow, not even CxtI itself.
   for (BasicBlock::const_iterator I(CxtI), IE(Inv); I != IE; ++I)
     if (!isGuaranteedToTransferExecutionToSuccessor(&*I))
       return false;
 
   return !isEphemeralValueOf(Inv, CxtI);
 }
 
+static bool isKnownNonZeroFromAssume(const Value *V, const Query &Q) {
+  // Use of assumptions is context-sensitive. If we don't have a context, we
+  // cannot use them!
+  if (!Q.AC || !Q.CxtI)
+    return false;
+
+  // Note that the patterns below need to be kept in sync with the code
+  // in AssumptionCache::updateAffectedValues.
+
+  auto CmpExcludesZero = [V](ICmpInst *Cmp) {
+    auto m_V = m_CombineOr(m_Specific(V), m_PtrToInt(m_Specific(V)));
+
+    Value *RHS;
+    CmpInst::Predicate Pred;
+    if (!match(Cmp, m_c_ICmp(Pred, m_V, m_Value(RHS))))
+      return false;
+    // Canonicalize 'v' to be on the LHS of the comparison.
+    if (Cmp->getOperand(1) != RHS)
+      Pred = CmpInst::getSwappedPredicate(Pred);
+
+    // assume(v u> y) -> assume(v != 0)
+    if (Pred == ICmpInst::ICMP_UGT)
+      return true;
+
+    // assume(v != 0)
+    // We special-case this one to ensure that we handle `assume(v != null)`.
+    if (Pred == ICmpInst::ICMP_NE)
+      return match(RHS, m_Zero());
+
+    // All other predicates - rely on generic ConstantRange handling.
+    ConstantInt *CI;
+    if (!match(RHS, m_ConstantInt(CI)))
+      return false;
+    ConstantRange RHSRange(CI->getValue());
+    ConstantRange TrueValues =
+        ConstantRange::makeAllowedICmpRegion(Pred, RHSRange);
+    return !TrueValues.contains(APInt::getNullValue(CI->getBitWidth()));
+  };
+
+  for (auto &AssumeVH : Q.AC->assumptionsFor(V)) {
+    if (!AssumeVH)
+      continue;
+    CallInst *I = cast<CallInst>(AssumeVH);
+    assert(I->getFunction() == Q.CxtI->getFunction() &&
+           "Got assumption for the wrong function!");
+    if (Q.isExcluded(I))
+      continue;
+
+    // Warning: This loop can end up being somewhat performance sensitive.
+    // We're running this loop for once for each value queried resulting in a
+    // runtime of ~O(#assumes * #values).
+
+    assert(I->getCalledFunction()->getIntrinsicID() == Intrinsic::assume &&
+           "must be an assume intrinsic");
+
+    Value *Arg = I->getArgOperand(0);
+    ICmpInst *Cmp = dyn_cast<ICmpInst>(Arg);
+    if (!Cmp)
+      continue;
+
+    if (CmpExcludesZero(Cmp) && isValidAssumeForContext(I, Q.CxtI, Q.DT))
+      return true;
+  }
+
+  return false;
+}
+
 static void computeKnownBitsFromAssume(const Value *V, KnownBits &Known,
                                        unsigned Depth, const Query &Q) {
   // Use of assumptions is context-sensitive. If we don't have a context, we
   // cannot use them!
   if (!Q.AC || !Q.CxtI)
     return;
 
   unsigned BitWidth = Known.getBitWidth();
 
   // Note that the patterns below need to be kept in sync with the code
if (MDNode *Ranges = Q.IIQ.getMetadata(I, LLVMContext::MD_range)) {
       if (auto *Ty = dyn_cast<IntegerType>(V->getType())) {
         const APInt ZeroValue(Ty->getBitWidth(), 0);
         if (rangeMetadataExcludesValue(Ranges, ZeroValue))
           return true;
       }
     }
   }
 
+  if (isKnownNonZeroFromAssume(V, Q))
+    return true;
+
   // Some of the tests below are recursive, so bail out if we hit the limit.
   if (Depth++ >= MaxDepth)
     return false;
 
   // Check for pointer simplifications.
   if (V->getType()->isPointerTy()) {
     // Alloca never returns null, malloc might.
     if (isa<AllocaInst>(V) && Q.DL.getAllocaAddrSpace() == 0)

llvm/test/Transforms/InstCombine/assume.ll

 ; Make sure the above canonicalization applies only
 ; to pointer types.  Doing otherwise would be illegal.
 
 define i1 @nonnull2(i32* %a) {
 ; CHECK-LABEL: @nonnull2(
 ; CHECK-NEXT:    [[LOAD:%.*]] = load i32, i32* [[A:%.*]], align 4
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ne i32 [[LOAD]], 0
 ; CHECK-NEXT:    tail call void @llvm.assume(i1 [[CMP]])
-; CHECK-NEXT:    [[RVAL:%.*]] = icmp eq i32 [[LOAD]], 0
-; CHECK-NEXT:    ret i1 [[RVAL]]
+; CHECK-NEXT:    ret i1 false
 ;
   %load = load i32, i32* %a
   %cmp = icmp ne i32 %load, 0
   tail call void @llvm.assume(i1 %cmp)
   %rval = icmp eq i32 %load, 0
   ret i1 %rval
 }
 
 ; Make sure the above canonicalization does not trigger
 ; if the assume is control dependent on something else
 
 define i1 @nonnull3(i32** %a, i1 %control) {
 ; CHECK-LABEL: @nonnull3(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[LOAD:%.*]] = load i32*, i32** [[A:%.*]], align 8
 ; CHECK-NEXT:    br i1 [[CONTROL:%.*]], label [[TAKEN:%.*]], label [[NOT_TAKEN:%.*]]
 ; CHECK:       taken:
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ne i32* [[LOAD]], null
 ; CHECK-NEXT:    tail call void @llvm.assume(i1 [[CMP]])
-; CHECK-NEXT:    [[RVAL:%.*]] = icmp eq i32* [[LOAD]], null
-; CHECK-NEXT:    ret i1 [[RVAL]]
+; CHECK-NEXT:    ret i1 false
 ; CHECK:       not_taken:
-; CHECK-NEXT:    ret i1 true
+; CHECK-NEXT:    [[RVAL_2:%.*]] = icmp sgt i32* [[LOAD]], null
+; CHECK-NEXT:    ret i1 [[RVAL_2]]
 ;
 entry:
   %load = load i32*, i32** %a
   %cmp = icmp ne i32* %load, null
   br i1 %control, label %taken, label %not_taken
 taken:
   tail call void @llvm.assume(i1 %cmp)
   %rval = icmp eq i32* %load, null
   ret i1 %rval
 not_taken:
-  ret i1 true
+  %rval.2 = icmp sgt i32* %load, null
+  ret i1 %rval.2
 }
 
 ; Make sure the above canonicalization does not trigger
 ; if the path from the load to the assume is potentially
 ; interrupted by an exception being thrown
 
 define i1 @nonnull4(i32** %a) {
 ; CHECK-LABEL: @nonnull4(
 ; CHECK-NEXT:    [[LOAD:%.*]] = load i32*, i32** [[A:%.*]], align 8
 ; CHECK-NEXT:    tail call void @escape(i32* [[LOAD]])
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp ne i32* [[LOAD]], null
 ; CHECK-NEXT:    tail call void @llvm.assume(i1 [[CMP]])
-; CHECK-NEXT:    [[RVAL:%.*]] = icmp eq i32* [[LOAD]], null
-; CHECK-NEXT:    ret i1 [[RVAL]]
+; CHECK-NEXT:    ret i1 false
 ;
   %load = load i32*, i32** %a
   ;; This call may throw!
   tail call void @escape(i32* %load)
   %cmp = icmp ne i32* %load, null
   tail call void @llvm.assume(i1 %cmp)
   %rval = icmp eq i32* %load, null
   ret i1 %rval

 
 ; PR37726 - https://bugs.llvm.org/show_bug.cgi?id=37726
 ; There's a loophole in eliminating a redundant assumption when
 ; we have conflicting assumptions. Verify that debuginfo doesn't
 ; get in the way of the fold.
 
 define void @debug_interference(i8 %x) {
 ; CHECK-LABEL: @debug_interference(
-; CHECK-NEXT:    [[CMP1:%.*]] = icmp eq i8 [[X:%.*]], 0
+; CHECK-NEXT:    [[CMP2:%.*]] = icmp ne i8 [[X:%.*]], 0
 ; CHECK-NEXT:    tail call void @llvm.dbg.value(metadata i32 5, metadata !7, metadata !DIExpression()), !dbg !9
-; CHECK-NEXT:    tail call void @llvm.assume(i1 [[CMP1]])
+; CHECK-NEXT:    tail call void @llvm.assume(i1 false)
 ; CHECK-NEXT:    tail call void @llvm.dbg.value(metadata i32 5, metadata !7, metadata !DIExpression()), !dbg !9
 ; CHECK-NEXT:    tail call void @llvm.dbg.value(metadata i32 5, metadata !7, metadata !DIExpression()), !dbg !9
-; CHECK-NEXT:    tail call void @llvm.assume(i1 false)
+; CHECK-NEXT:    tail call void @llvm.assume(i1 [[CMP2]])
 ; CHECK-NEXT:    ret void
 ;
   %cmp1 = icmp eq i8 %x, 0
   %cmp2 = icmp ne i8 %x, 0
   tail call void @llvm.assume(i1 %cmp1)
   tail call void @llvm.dbg.value(metadata i32 5, metadata !1, metadata !DIExpression()), !dbg !9
   tail call void @llvm.assume(i1 %cmp1)
   tail call void @llvm.dbg.value(metadata i32 5, metadata !1, metadata !DIExpression()), !dbg !9

llvm/test/Transforms/InstSimplify/assume-non-zero.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -instsimplify -S | FileCheck %s
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 declare void @llvm.assume(i1) #1
 
 define i1 @nonnull0_true(i8* %x) {
 ; CHECK-LABEL: @nonnull0_true(
 ; CHECK-NEXT:    [[A:%.*]] = icmp ne i8* [[X:%.*]], null
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[A]])
-; CHECK-NEXT:    [[Q:%.*]] = icmp ne i8* [[X]], null
-; CHECK-NEXT:    ret i1 [[Q]]
+; CHECK-NEXT:    ret i1 true
 ;
   %a = icmp ne i8* %x, null
   call void @llvm.assume(i1 %a)
   %q = icmp ne i8* %x, null
   ret i1 %q
 }
 
 define i1 @nonnull1_true(i8* %x) {
 ; CHECK-LABEL: @nonnull1_true(
 ; CHECK-NEXT:    [[INTPTR:%.*]] = ptrtoint i8* [[X:%.*]] to i64
 ; CHECK-NEXT:    [[A:%.*]] = icmp ne i64 [[INTPTR]], 0
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[A]])
-; CHECK-NEXT:    [[Q:%.*]] = icmp ne i8* [[X]], null
-; CHECK-NEXT:    ret i1 [[Q]]
+; CHECK-NEXT:    ret i1 true
 ;
   %intptr = ptrtoint i8* %x to i64
   %a = icmp ne i64 %intptr, 0
   call void @llvm.assume(i1 %a)
   %q = icmp ne i8* %x, null
   ret i1 %q
 }
 
 define i1 @nonnull2_true(i8 %x, i8 %y) {
 ; CHECK-LABEL: @nonnull2_true(
 ; CHECK-NEXT:    [[A:%.*]] = icmp ugt i8 [[X:%.*]], [[Y:%.*]]
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[A]])
-; CHECK-NEXT:    [[Q:%.*]] = icmp ne i8 [[X]], 0
-; CHECK-NEXT:    ret i1 [[Q]]
+; CHECK-NEXT:    ret i1 true
 ;
   %a = icmp ugt i8 %x, %y
   call void @llvm.assume(i1 %a)
   %q = icmp ne i8 %x, 0
   ret i1 %q
 }
+define i1 @nonnull2_true_swapped(i8 %x, i8 %y) {
+; CHECK-LABEL: @nonnull2_true_swapped(
+; CHECK-NEXT:    [[A:%.*]] = icmp ult i8 [[Y:%.*]], [[X:%.*]]
+; CHECK-NEXT:    call void @llvm.assume(i1 [[A]])
+; CHECK-NEXT:    ret i1 true
+;
+  %a = icmp ult i8 %y, %x
+  call void @llvm.assume(i1 %a)
+  %q = icmp ne i8 %x, 0
+  ret i1 %q
+}
 
 define i1 @nonnull3_unknown(i8 %x) {
 ; CHECK-LABEL: @nonnull3_unknown(
 ; CHECK-NEXT:    [[Q:%.*]] = icmp ne i8 [[X:%.*]], 0
 ; CHECK-NEXT:    ret i1 [[Q]]
 ;
   %a = icmp uge i8 %x, 0
   call void @llvm.assume(i1 %a)
   %q = icmp ne i8 %x, 0
   ret i1 %q
 }
 define i1 @nonnull4_true(i8 %x) {
 ; CHECK-LABEL: @nonnull4_true(
 ; CHECK-NEXT:    [[A:%.*]] = icmp uge i8 [[X:%.*]], 1
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[A]])
-; CHECK-NEXT:    [[Q:%.*]] = icmp ne i8 [[X]], 0
-; CHECK-NEXT:    ret i1 [[Q]]
+; CHECK-NEXT:    ret i1 true
 ;
   %a = icmp uge i8 %x, 1
   call void @llvm.assume(i1 %a)
   %q = icmp ne i8 %x, 0
   ret i1 %q
 }
 
 define i1 @nonnull5_unknown(i8 %x) {
 ; CHECK-LABEL: @nonnull5_unknown(
 ; CHECK-NEXT:    [[A:%.*]] = icmp sgt i8 [[X:%.*]], -1
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[A]])
 ; CHECK-NEXT:    [[Q:%.*]] = icmp ne i8 [[X]], 0
 ; CHECK-NEXT:    ret i1 [[Q]]
 ;
   %a = icmp sgt i8 %x, -1
   call void @llvm.assume(i1 %a)
   %q = icmp ne i8 %x, 0
   ret i1 %q
 }
 define i1 @nonnull6_true(i8 %x) {
 ; CHECK-LABEL: @nonnull6_true(
 ; CHECK-NEXT:    [[A:%.*]] = icmp sgt i8 [[X:%.*]], 0
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[A]])
-; CHECK-NEXT:    [[Q:%.*]] = icmp ne i8 [[X]], 0
-; CHECK-NEXT:    ret i1 [[Q]]
+; CHECK-NEXT:    ret i1 true
 ;
   %a = icmp sgt i8 %x, 0
   call void @llvm.assume(i1 %a)
   %q = icmp ne i8 %x, 0
   ret i1 %q
 }
 define i1 @nonnull7_true(i8 %x) {
 ; CHECK-LABEL: @nonnull7_true(
 ; CHECK-NEXT:    [[A:%.*]] = icmp sgt i8 [[X:%.*]], 1
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[A]])
-; CHECK-NEXT:    [[Q:%.*]] = icmp ne i8 [[X]], 0
-; CHECK-NEXT:    ret i1 [[Q]]
+; CHECK-NEXT:    ret i1 true
 ;
   %a = icmp sgt i8 %x, 1
   call void @llvm.assume(i1 %a)
   %q = icmp ne i8 %x, 0
   ret i1 %q
 }
 
 define i1 @nonnull8_unknown(i8 %x) {
;
   call void @llvm.assume(i1 %a)
   %q = icmp ne i8 %x, 0
   ret i1 %q
 }
 define i1 @nonnull10_true(i8 %x) {
 ; CHECK-LABEL: @nonnull10_true(
 ; CHECK-NEXT:    [[A:%.*]] = icmp sge i8 [[X:%.*]], 1
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[A]])
-; CHECK-NEXT:    [[Q:%.*]] = icmp ne i8 [[X]], 0
-; CHECK-NEXT:    ret i1 [[Q]]
+; CHECK-NEXT:    ret i1 true
 ;
   %a = icmp sge i8 %x, 1
   call void @llvm.assume(i1 %a)
   %q = icmp ne i8 %x, 0
   ret i1 %q
 }
 
 define i1 @nonnull11_true(i8 %x) {