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

[Hal Finkel via llvm-commits]

On 09/03/2017 06:38 PM, Sanjoy Das via llvm-commits wrote:
> Hi,
>
> +CC llvm-commits
>
> On Sun, Sep 3, 2017 at 4:01 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
>> On Sun, Sep 3, 2017 at 12:09 PM, Sanjoy Das via Phabricator via llvm-commits
>> <llvm-commits at lists.llvm.org> wrote:
>>> sanjoy requested changes to this revision.
>>> sanjoy added a comment.
>>> This revision now requires changes to proceed.
>>>
>>> Hi David,
>>>
>>> Generally it is not safe to look through `inttoptr` and `ptrtoint` in
>>> SCEV.
>>> https://github.com/llvm-mirror/llvm/blob/master/lib/Analysis/ScalarEvolution.cpp#L6053
>>> has a little bit of the rationale, but basically the problem is that in:
>>>
>>>    %i0 = ptrtoint i8* %p0 to i64
>>>    %i1 = ptrtoint i8* %p1 to i64
>>>    %i2 = add i32 %i0, %i1
>>>    %ptr = inttoptr i64 %i2 to i8*
>>>
>>> `%ptr` can alias both `%p0` and `%p1`.
>>
>> Okay, AFAICT, given p0 noalias p1, it can really alias the disjunction of p0
>> and p1, right?
>>
>> IE either p0 is null or p1 is null, and then it aliases the other one. It
>> doesn't really alias both at the same time, it's just you can't tell which
>> of them it aliases statically.
> Yes.  I should have said "`%ptr` is may-alias with both `%p0` and
> `%p1`" which would have made this clearer.
>
>>> However, if you round-trip `%ptr` through SCEV and SCEVExpander, then
>>> nothing stops SCEVExpander from expanding the SCEV for `%ptr` to
>>>
>>>    %i0 = ptrtoint i8* %p0 to i64
>>>    %i1 = ptrtoint i8* %p1 to i64
>>>    %p0_ = inttoptr i64 %i0 to i8*
>>>    %ptr = getelementptr i8, i8* %p0_, i64 %p0
>>>
>> I assume you meant to index it by  something else, since this index is not
>> an i64?
> Yes, as David already pointed out, this should have been `%ptr =
> getelementptr i8, i8* %p0, i64 %i1` (or `%ptr = getelementptr i8, i8*
> %p0_, i64 %i1`, both expansions have the same problem).
>
>> I also don't understand "  // It's tempting to handle inttoptr and ptrtoint
>> as no-ops, however this can
>>    // lead to pointer expressions which cannot safely be expanded to GEPs,
>>    // because ScalarEvolution doesn't respect the GEP aliasing rules when
>>    // simplifying integer expressions."
>>
>> 1. What are "the gep aliasing rules", exactly?
>> https://llvm.org/docs/LangRef.html#pointeraliasing does not mention special
>> rules for gep :)
> I think the rule it is talking about is that reads and writes on a GEP
> are allowed to only read from and write to the allocation that is
> passed in as the first parameter to the GEP -- you can't GEP from one
> allocation to another different allocation and issue reads/writes.  So
> my statement above was imprecise, `%p0` no-alias `%p1` is not
> sufficient to infer no-alias in the GEP case, you'd have to know that
> `%p0` and `%p1` are different allocations (based off different malloc
> or alloca, for instance).
>
> I think the pointer aliasing rules have a typo (which I'll fix), but
> in principle this follows from:
>
> "Any memory access must be done through a pointer value associated
> with an address range of the memory access, otherwise the behavior is
> undefined. Pointer values are associated with address ranges according
> to the following rules"
>
> "A pointer value is associated with the addresses associated with any
> value it is based on."
>
> "A pointer value formed from a getelementptr operation is based on the
> second [should be *first*] value operand of the getelementptr."

I don't think it is a typo. It is referring to the textual form where we 
now print the type first and the base pointer second.

  -Hal

>
>> 2.  Even ignoring "the gep aliasing rules" i don't see how this is ever a
>> correct expansion of p0.
> Yes, that's a copy/paste typo on my part.  I should have used %i1 as the index.
>
> -- Sanjoy
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory