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

[Quuxplusone]

Bugzilla Link	PR11241
Status	NEW
Importance	P normal
Reported by	Richard Smith (richard-llvm@metafoo.co.uk)
Reported on	2011-10-26 16:10:25 -0700
Last modified on	2012-12-03 18:40:56 -0800
Version	trunk
Hardware	PC Linux
CC	baldrick@free.fr, chandlerc@gmail.com, geek4civic@gmail.com, llvm-bugs@lists.llvm.org, nicholas@mxc.ca
Fixed by commit(s)
Attachments	cmp.ll (592 bytes, application/octet-stream)
Blocks
Blocked by
See also

Created attachment 7536
Testcase

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.

[Quuxplusone]

Attached cmp.ll (592 bytes, application/octet-stream): Testcase

[Quuxplusone]

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?

[Quuxplusone]

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

[Quuxplusone]

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

[Quuxplusone]

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?

[Quuxplusone]

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<ConstantPointerNull>(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?

[Quuxplusone]

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
  ...