Re: [range-ops] patch 01/04: types for VR_UNDEFINED and VR_VARYING

[Andrew MacLeod <amacleod@redhat.com>]

On 8/1/19 10:11 AM, Richard Biener wrote:
> On Thu, Aug 1, 2019 at 1:24 AM Andrew MacLeod <amacleod@redhat.com> wrote:
>
>> Aldy started this relatively straightforward submission a month ago (!!
>> July 1st!), and we've made virtually no progress since then.
> Actually I think it's a waste of time on my side since I actually sat
> down and looked at the branch - because while you posted several
> "merge plans / proposals" the actual patches you proposed for integration
> were just changes to the existing code.  But then when I tried to

They are preliminary patches which enable us to align irange and 
value_range so they can be used interchangeably as the range class 
everywhere. We've adjusted irange on the branch to align with 
value_range, and these tweaks to the value_range API make them fully 
interchangeable.

So yes, this first set of patches are just changes to the existing 
code... We need them before we can bring in range-ops.    After that 
comes the outgoing range computational component, but I'm worried we're 
short on time for that now :-(  we'll see.



> understand why you need those by looking at the branch (OK, criticizing
> some stuff I saw there) you said "well, don't look - it's all just a prototype
> and even the APIs are not finished".
I explained numerous times why we needed the changes, you appear to 
simply not believe me. There should not have been a need to look at 
unprepared code.

As for being not finished, the API is being tweaked for trunk.. 2 things 
in particular:

  1)  I'm adding a type to the calls since we won't have a type for 
undefined any more. The aggregation of sub-ranges always started with 
undefined and built up from there, and it can no longer find the type 
from undefined.
  2)  The Implementation of each operation was still using chains of 
calls into legacy code which often used big switches...  I'm collapsing 
those into a basic operation call on wide-ints  which is being added to 
the API.    This will dramatically change the look of the file, without 
changing its basic behaviour.

So its virtually the same code, but with a very different (and cleaner) 
look designed for trunk...  we didn't really care what it looked like on 
the branch, we just wanted functionality...  and it shows.  Hence my 
desire to not have it critiqued :-)  The intention was always to rework 
it for trunk before having it examined.

I'll also modify the entire file to use value_range instead of irange.  
We don't want to contaminate trunk with pointless #defines or other 
silliness. We'll manage irange compatibility for range-ops on the 
branch. The varying min/max change we were trying to get in is a hard 
prerequisite for value_range to function properly in this role.


> That makes me wonder why you do changes to trunk or proposing
> merge plans when you've not even finished designing stuff...
>
> As said, I feel I am wasting my time here so please feel free to
> do whatever you are up to with VRP.
>
> I did help with considerable resources even during last stage1/3 with
> "fixing" the half-way conversion of value_range to a class.  I've not
> manged to beat sense into the wide-int-range.h API, but oh well.
yes, and I really do appreciate that work, it was critical to paving the 
way to allowing us to use value_range instead of irange...  We just need 
these final couple of things to finish that transition, and they were 
just not happening, thus my reaction.

I think the new  range-op operation on wide-ints that is being added to 
the API will probably replace most, if not all, of the wide-int-range.h 
stuff...  We'll see how it goes when I finally get back to finishing it.

Anyway, I'm off after today for a week, so we wont do anything further 
until I get back.
The weekend will be here shortly... Have a good one.

Andrew