[PATCH] D37419: Teach scalar evolution to handle inttoptr/ptrtoint

[Nuno Lopes via llvm-commits]

(just got back from vacations; slowly catching up on emails).

I don't have much to add to what Sanjoy already said. Just wanted to second 
that a round-trip of inttoptr/ptrtoint is not a NOP!
See this corner case, for example:
a = malloc(n)
b = malloc(..)
v = ptrtoint(a + n)
w = ptrtoint(b)
if (v == w)
   p = inttoptr(w)
   *p = ...

Now imagine GVN replaces w with v:
v = ptrtoint(a + n)
w = ptrtoint(b)
if (v == w)
   p = inttoptr(v)
   *p = ...

And imagine that we consider the round-trip as a nop and optimize 
accordingly:
if (v == w)
   p = a+n
   *p = ...

The dereference of p now became UB. The original program was well defined, 
though, because it was accessing the buffer at 'b', not at 'a'.
It's a simple example, but shows the problem of consider the round-trip as a 
nop.

We are trying to get some consistent model for LLVM, but it's taking some 
time.. :)  We already have a 30-page document with problems, solutions, 
etc.. We'll share it once it's a bit more stable.

Nuno


-----Original Message----- 
From: Sanjoy Das via llvm-commits
Sent: Monday, September 4, 2017 8:01 AM
Subject: Re: [PATCH] D37419: Teach scalar evolution to handle 
inttoptr/ptrtoint

Hi all,

This is a "batched" reply:

On Sun, Sep 3, 2017 at 6:35 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
> I'll also point out, you didn't read or write anything in your example.
> Which is strange to me because the usual semantic is "allowed to alias, 
> but
> not allowed to be used to load/store".
> IE if you ask if the two pointers alias, the answer is yes.
> If you ask if a load can touch a given store, the answer is no.
>
> Here you are saying the pointer aliasing rules apply even to
> unwritten/unread pointers.
> That to me, is ... very strange.

I agree that it is odd to assign aliasing relationship between
pointers, and that isn't central to what I was trying to say.  The
behavior remains if I add a load and a store:

```
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"

define void @f(i64 %arg, i64 %arg1) {
bb:
  %p0 = tail call noalias i8* @malloc(i64 %arg) #3
  %p1 = tail call noalias i8* @malloc(i64 %arg) #3
  %offset = tail call i64 @subtract(i8* %p1, i8* %p0)
  %gep = getelementptr i8, i8* %p0, i64 %offset  ;; not inbounds
  %v0 = load i8, i8* %gep
  store i8 40, i8* %p1
  ret void
}

declare noalias i8* @malloc(i64)
declare i64 @subtract(i8*, i8*)
```

opt -basicaa -aa-eval -print-all-alias-modref-info
-evaluate-aa-metadata ir.ll |& grep NoAlias
  NoAlias:      i8* %p0, i8* %p1
  NoAlias:      i8* %gep, i8* %p1
  NoAlias:   %v0 = load i8, i8* %gep <->   store i8 40, i8* %p1

On Sun, Sep 3, 2017 at 10:26 PM, Xinliang David Li <davidxl at google.com> 
wrote:
> SCEV itself can be canonicalized plus one bit to show it if is commutable.
> If it is, generate the conservative/aliasing losing add form, otherwise
> generate the GEP form.

Yes, something like that can work; but at that point I'd rather have
ptrtoint and inttoptr be explicitly modeled in SCEV.

> However I find this situation extremely rare to be worth the effort: a
> really deferencible pointer is used as an offset. If the pointer is not
> dereferenced, the false aliasing does not really matter.

I may not have fully grasped what you're suggesting (so what I'm about
to say may be off base), but if a SCEV->SCEV expander round-trip has
to transform:

```
%a0 = alloca
%a1 = alloca
escape(%a1)
%offset = f()
%ptr = gep %a0, %offset
```

to

```
%a0 = alloca
%a1 = alloca
escape(%a1)
%offset = f()
%i0 = ptrtoint %a0
%ptr.int = add %i0, %offset
%ptr = inttoptr %ptr.int
```

to be correct in possible but rare (pointer adding) cases, then we
lose the fact that %ptr no-alias %a1.  IOW compensating for the rare
but possible case can regress aliasing for the normal cases.

On Sun, Sep 3, 2017 at 11:40 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
> So, i instrumented llvm to add runtime asserts about the noaliasing 
> pointers
> before free (IE it adds an assert that, for each pointer that is noaliased
> with the pointer passed to free, free is not destroying that pointer), and
> it fails really quickly.

That's not too surprising; a simple case where this may happen is:

  %p0 = malloc(10)
  free(%p0)
  %p1 = malloc(10);
  free(%p1)

In the second free %p0 no-alias %p1, but they're very likely the same
bitwise value.  NoAlias does not mean "different bitwise values".

Having said that, there are known issues with how we model memory in
LLVM.  Nuno (CC'ed), Gil, John et. al. are working on fixing this.

-- Sanjoy