[llvm-commits] [pr12979][patch/rfc] Clear nsw/nuw in gvn

[Nuno Lopes]

> On Fri, Jun 1, 2012 at 6:39 PM, Nuno Lopes <nunoplopes at sapo.pt> wrote:
>> Hi,
>>
>>> On 30 May 2012 12:16, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>>>>> This seems right.  For range metadata you should form the union of the
>>>>> ranges I guess.
>>>>
>>>> Implemented.
>>>
>>> I have rebased it now that the verifier enforces that the range is in
>>> a canonical form. I have also fixed corner cases like a merge forming
>>> the full set.
>>
>>
>> Although I'm arriving a bit late to the discussion, let me just write
>> my thoughts on this.
>> I think that with range metadata we can be more aggressive. For
>> example, if you have the following:
>> %a = load i32* %p, !range !1
>> %b = load i32* %p, !range !2
>>
>> I think the resulting range should be the *intersection*, and not the
>> union of the ranges.  Someone says: "I'm sure this value is between 0
>> and 5", and someone else says "I'm sure that value is between 3 and
>> 6".  Then we know that the value must be between 3 and 5;  we don't
>> need to expand our beliefs to be between 0 and 6.
>> (of course this reasoning assumes that ranges are always conservative,
>> which must be the case, anyway)
>>
> This is right.
>
>> For TBAA info, I think the same reasoning applies. I think we can pick
>> the strongest aliasing information.
>
> This is wrong :)
> First, there is no such thing as "strongest". TBAA is a tree
> structure, and all your ancestors and descendants may-alias.
> At least in C, the accesses should be through things that are in the
> same tree anyway.
> Choosing different members of that same TBAA subtree to represent the
> sets would not change anything.
>
> Ignoring this neither LLVM (nor C++, for that matter) guarantee that
> the same *place in memory* is not referred to with disjoint TBAA
> types.  C++ for example, has a placement new that allows type
> changing.
>
> So it's perfectly legal (at least by the docs) to have a TBAA tree like this:
>
>
>    0
>  /   \
> 1   2
> |
> 3
> and
>
> %a = load i32* %p, !tbaa !1
> %b = load i32* %p, !tbaa !2
>
> While we can eliminate the second load, it would be quite wrong to say
> that suddenly this !1 load over here is, for alias analysis purposes,
> a !2 load because !2 is "stronger" since the tree is smaller.    They
> aren't in the same TBAA subtree, and you are suddenly saying a whole
> class of dependents you don't know about dont conflict.
>
> In short: TBAA is not may-alias info, it's type info. If it was
> may-alias info, yes, you could have "anti-alias sets" and say that
> something didn't conflict with something else.  But without knowing
> the original program language semantics, you can't go willy-nilly
> changing the tbaa info :)

You're very right!  I was confusing this TBAA business.
So as far as I understand, the optimal solution is to find the lowest  
common ancestor, e.g.:

     0
    /
   1
  / \
2   3

merging !2 and !3 should yield !1  (meaning it can alias both !2 and  
!3 sub-trees types).


In the (unlikely) case they don't have the same root, then something  
is really wrong, and probably it's some sort of undefined behavior.

The proposed patch is not so aggressive. It will gracefully fail if  
one of the types is not a descendent of the other.

Nuno