Re: [PATCH 04/10] tree-object-size: Single pass dependency loop resolution

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 11/23/21 17:37, Jakub Jelinek wrote:
> On Wed, Nov 10, 2021 at 12:31:30AM +0530, Siddhesh Poyarekar wrote:
>> Use SSA names as placeholders self-referencing variables to generate
>> expressions for object sizes and then reduce those size expressions
>> to constants instead of repeatedly walking through statements.
>>
>> This change also makes sure that object sizes for an SSA name are
>> updated at most twice, once if there is a dependency loop and then the
>> final time upon computation of object size.  Iteration to deduce the
>> final size is now done on the size expressions instead of walking
>> through the object references.
>>
>> Added test to include a case where __builtin_object_size incorrectly
>> returned the minimum object size as zero.
> 
> This feels way too risky to me.  I think making some code do something
> different between (x & OST_DYNAMIC) == 0 and == 1 is just fine,
> it doesn't have to share everything.  After all, for __bdos we actually
> emit code to compute it at runtime, while for __bos we don't.
> So I'd keep e.g. .pass = 0, .pass = 1 and .pass = 2 (in a loop) in
> compute_builtin_object_size for non-OST_DYNAMIC and only use your new
> stuff for __bdos.
> E.g. creating new SSA_NAMEs for __bos doesn't look like a good idea to me.
> The GCC __bos testsuite is not big enough that it covers everything and
> even in it we sometimes reexamine 2, 3 or 4 times.

OK, so addr_object_size does not participate in dependency loops, so I 
can keep its changes intact and simply add a INTEGER_CST check at the 
end to return unknown if the size expression is not constant; we kinda 
do that for object offsets anyway.

I could then make a new entry point collect_object_sizes_for (renaming 
the current one to collect_static_object_sizes_for and a new one 
collect_dynamic_object_sizes_for, routing based on object_size_type & 
OST_DYNAMIC) which sends the object size collection down different 
paths.  I can reuse the object size vectors and just have different data 
in them.

Would that be reasonable?

Thanks,
Siddhesh