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

Duncan Sands baldrick at free.fr
Sat Jun 2 01:36:13 PDT 2012


Hi Danny,

On 02/06/12 01:42, Daniel Berlin wrote:
> 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.

I think this is wrong.  Consider something like this, where p is a void*
that points either to some enum (enum E) or to a general i32, depending
on the value of a flag:

    int t;
    if (points_to_enum)
      t = (int)(*(Enum E*)p));  // load E
    else
      t = *(int*)p;             // load int

The first load (load E) will get range metadata, the second load will not.
(More generally, the second load could be of some other enum type with a
wider range).  At the IR level the two loads are identical, and of the same
pointer, thus GVN will (probably) merge them.  However it would be wrong to
tag the merged load with the range data for E, since it might have been a
load of the general int value.  Probably you can get the same kind of thing
using unions.

Ciao, Duncan.



More information about the llvm-commits mailing list