[llvm-commits] [patch][pr12251] Add range metadata to llvm. Make clang produce it.

[Duncan Sands]

Hi Rafael,

>>> I think you should have the ranges map exactly to the ConstantRange class,
>>> otherwise it is confusing to have more than one range concept inside the
>>> compiler (and of course being able to use the ConstantRange methods directly
>>> is convenient too).  As such ranges are open at the end, so { i8 0, i8 2 }
>>> means "Can only be 0 or 1".
>>
>> That class has to be able to handle empty ranges, so it has some funny
>> special cases. If I understand it correctly, with i8 the range [x, x)
>> is empty for any x except 255 in which case it is the full range.

I don't think that's a reason to not use the ConstantRange class, it is a
reason to have the verifier reject attempts to form an empty or a full range
though.  So I think you should have ranges be [A, B) but explain that empty
and full ranges are not allowed.  Indeed forming one of these probably means
that the front-end writer made a mistake.

Ciao, Duncan.

>>
>> With closed ranges the empty range is unrepresentable, which is
>> probably a good thing. I this patch I have added a check to the
>> verifier to reject full ranges too. I don't have a strong opinion
>> about this, so I am fine with moving to open ranges if other prefer
>> it.
>>
>> On the clang side the things I have changed are
>> * Don't output the range when an enum can take any value (would be the
>> full range).
>> * Don't output the range for C enums
>> * Use the desugared type to look for enums.
>>
>> Cheers,
>> Rafael