Re: VRP: abstract out wide int CONVERT_EXPR_P code

[Michael Matz <matz at suse dot de>]

Hi,

On Wed, 5 Sep 2018, Aldy Hernandez wrote:

> No apologies needed.  I welcome all masochists to join me in my personal hell
> :).

;-)

> > How can this test and following code catch all problematic cases?  Assume
> > a range of [253..257], truncating to 8 bits unsigned.  The size of the
> > range is 5 (not 4 as the code above calculates), well representable in 8
> > bits.  Nevertheless, the min of the new range isn't 253, nor is the max of
> > the new range 257.  In fact the new range must be [0..255] (if you have no
> > multi ranges or anti-ranges).  So whatever this function is supposed to
> > do (what btw?), it certainly does not convert a range.
> 
> Welcome to the wonderful world of anti ranges, where nothing is as it 
> seems.

Yes, as I said, to precisely capture the result of '[253..257] & 255' you 
either need multi-ranges or an anti-range.  

> First you're not looking at what's currently in mainline.  Richard 
> approved the first incantation of this code.  But even so, I believe the 
> above is correct as well.

Well, maybe as part of a larger sequence of code, but the above code 
doesn't even return [253..1], there's actually no conversion/change of the 
inputs done at all.  Yes, the current code actually does the truncation.  
I guess I was triggered by the name range_convert that didn't actually do 
any conversion.  I now have no issue with the code in mainline anymore 
(well, maybe except disliking functions with eight parameters).

> If desired, I could further document that the range returned may have 
> its contents swapped and that is why it needs canonizing.  But hey, that 
> shit was already broken when I got here ;-).
> 
> If you follow the code in tree-vrp.c before my patch you will see that 
> the path is the same... we calculate a nonsensical range of [253, 1] and 
> then massage it into ~[2, 252] in set_and_canonicalize_value_range.

Yes, I saw that, I think I was sceptical about splitting off a part of the 
code that wasn't self-sufficient.

> I think the fact that it's so hard to get all this right, is a strong 
> argument for getting rid of anti ranges.  Almost all the bugs or 
> oversights I've fixed over the past months in VRP have been related to 
> anti ranges.

Well, without anti-ranges you need multi-ranges (or at least two-ranges); 
it's a tradeoff.  If I'd have to choose a high-level interface and the 
choice is between supporting two-ranges directly (as two parameters) or a 
single range/anti-range I'd always take the latter (internally of course 
it would be represented as a two-range to make arguing and implementing 
stuff easier).


Ciao,
Michael.