On 5/15/23 20:14, Aldy Hernandez wrote:
> 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.

I've adapted the patch to GCC13 and tested it on x86-64 Linux.  Please 
look over the new[] I do for trees to make sure I did things right.

int_range_max on GCC13 is currently 4112 bytes.  Here are the numbers 
for various defaults:

                 < 2> =  64 bytes, 3.02% for VRP.
                 < 3> =  80 bytes, 2.67% for VRP.
                 < 8> = 160 bytes, 2.46% for VRP.
                 <16> = 288 bytes, 2.40% for VRP.

Note that we don't have any runway on GCC13, so this would be a net loss 
in performance for VRP.  Threading shows about half as much of a drop 
than VRP.  Overall compilation is within 0.2%, so not noticeable.

I'm surprised 2 sub-ranges doesn't incur a  bigger penalty, but 3 seems 
to be the happy medium.  Anything more than that, and there's no difference.

The patch defaults to 3 sub-ranges.  I must say, 80 bytes looks mighty 
nice.  It's up to you what to do with the patch.  I'm chicken shit at 
heart and hate touching release compilers :).

Aldy