known constant not folded: new() + 4 != 0

[zygoloid]

Bugzilla Link	11241
Version	trunk
OS	Linux
Attachments	Testcase
CC	@chandlerc,@nlewycky

Extended Description

This IR:

  %alloc2 = tail call noalias i8* @​_Znwm(i64 8) nounwind
  %add = getelementptr inbounds i8* %alloc2, i64 4
  %always_false = icmp eq i8* %add, null
  ret i1 %always_false

... is collapsed to "ret i1 0". However, this IR:

  %alloc2 = tail call noalias i8* @​_Znwm(i64 8) nounwind
  %add = getelementptr inbounds i8* %alloc2, i64 4
  %always_false = icmp eq i8* %add, null
  br i1 %always_false, label %skipinit2, label %init2

init2:                                            ; preds = %entry
  store i8 1, i8* %alloc2, align 1
  ret void

skipinit2:                                        ; preds = %entry
  ret void

... is not optimized any further.

[nlewycky]

By contrast, this testcase:

declare i8* @​_Znwm(i64)
declare void @​_ZdlPv(i8*)

define void @​test() {
%alloc2 = tail call noalias i8* @​_Znwm(i64 8) nounwind
%add = getelementptr inbounds i8* %alloc2, i64 4
%always_false = icmp eq i8* %add, null
br i1 %always_false, label %skipinit2, label %init2
init2: ; preds = %entry
store i8 1, i8* %alloc2, align 1
call void @​_ZdlPv(i8* %alloc2)
ret void
skipinit2: ; preds = %entry
call void @​_ZdlPv(i8* %alloc2)
ret void
}

works fine because DSE sees the delete call and eliminates the dead store, then instcombine's "pretend malloc always succeeds" logic can kick in.

Given that, is this reduced testcase still worth pursuing?

[zygoloid]

Probably not. It's a pity that DSE can't squish the store here (presumably it thinks the pointer escapes via the comparison result).

[nlewycky]

Right, to DSE the comparison test looks like a capture and to instcombine the store looks like a use.

[llvmbot]

I thought capture analysis (at least how CaptureTracking.h does it) tries to be
understanding about comparisons with null. Does DSE do its own capture analysis?

[nlewycky]

Hunh, you're right. I didn't know it was supposed to. It's getting tripped up by the GEP, here:

128 case Instruction::ICmp:
129 // Don't count comparisons of a no-alias return value against null as
130 // captures. This allows us to ignore comparisons of malloc results
131 // with null, for example.
132 if (isNoAliasCall(V->stripPointerCasts()))
133 if (ConstantPointerNull *CPN =
134 dyn_cast(I->getOperand(1)))
135 if (CPN->getType()->getAddressSpace() == 0)
136 break;

V is a GEPI, V->stripPointerCasts() is still the GEPI, so "isNoAliasCall" fails.

llvm::GetUnderlyingObject(V, UNLL, 100) returns the malloc call though. Maybe we should use that instead?

[llvmbot]

Why is this code testing that the result plus 4 is not null, rather than the
result? One nasty issue here is that you can capture a pointer by offsetting it
by constants (eg using GEP) and comparing with null. Here there is just one
comparison, but I could write 2^64 comparisons (!) and cunning capture the value
of the pointer, essentially by doing this:

c0 = imcp eq ptr, 0
br c0, ret0, try1
ret0:
ret 0
try1:
p1 = add ptr, -1
c1 = icmp eq ptr, 0
br c1, ret1, try2
ret1:
ret 1
try2:
p2 = add ptr, -2
...

[philnik777]

Closing, since the reproducer doesn't work anymore.