Re: [PATCH] Add auto-resizing capability to irange's [PR109695]

[Aldy Hernandez <aldyh@redhat.com>]

On 5/15/23 17:07, Aldy Hernandez wrote:
> 
> 
> On 5/15/23 12:42, Jakub Jelinek wrote:
>> On Mon, May 15, 2023 at 12:35:23PM +0200, Aldy Hernandez wrote:
>>> gcc/ChangeLog:
>>>
>>>     PR tree-optimization/109695
>>>     * value-range.cc (irange::operator=): Resize range.
>>>     (irange::union_): Same.
>>>     (irange::intersect): Same.
>>>     (irange::invert): Same.
>>>     (int_range_max): Default to 3 sub-ranges and resize as needed.
>>>     * value-range.h (irange::maybe_resize): New.
>>>     (~int_range): New.
>>>     (int_range::int_range): Adjust for resizing.
>>>     (int_range::operator=): Same.
>>
>> LGTM.
>>
>> One question is if we shouldn't do it for GCC13/GCC12 as well, perhaps
>> changing it to some larger number than 3 when the members aren't 
>> wide_ints
>> in there but just trees.  Sure, in 13/12 the problem is 10x less severe
>> than in current trunk, but still we have some cases where we run out of
>> stack because of it on some hosts.
> 
> Sure, but that would require messing around with the gt_* GTY functions, 
> and making sure we're allocating the trees from a sensible place, etc 
> etc.  I'm less confident in my ability to mess with GTY stuff this late 
> in the game.

Hmmm, maybe backporting this isn't too bad.  The only time we'd have a 
chunk on the heap is for int_range_max, which will never live in GC 
space.  So I don't think we need to worry about GC at all.

Although, legacy mode in GCC13 does get in a the way a bit.  Sigh.

And unrealted, but speaking of GC... we should remove all GTY markers 
from vrange.  It should never live in GC.  That's why we have 
vrange_storage for, and that is what we put in the tree_ssa_name.

  /* Value range information.  */
   union ssa_name_info_type {
     /* Range and aliasing info for pointers.  */
     struct GTY ((tag ("0"))) ptr_info_def *ptr_info;
     /* Range info for everything else.  */
     struct GTY ((tag ("1"))) vrange_storage * range_info;
   } GTY ((desc ("%1.typed.type ?" \
                 "!POINTER_TYPE_P (TREE_TYPE ((tree)&%1)) : 2"))) info;

That should have been the only use of range GC stuff, but alas another 
one crept in... IPA:

struct GTY (()) ipa_jump_func
{
...
   /* Information about value range, containing valid data only when 
vr_known is
      true.  The pointed to structure is shared betweed different jump
      functions.  Use ipa_set_jfunc_vr to set this field.  */
   value_range *m_vr;
...
};

This means that we can't nuke int_range<N> and default to an always 
resizable range just yet, because we'll end up with the value_range in 
GC memory, and resizable part in the heap.

That m_vr pointer should be a pointer to vrange_storage.  Meh...I'm 
bumping against my IPA work yet again.  I think it's time to start 
dusting off those patches.

Aldy