Re: [PATCH 04/10] tree-object-size: Single pass dependency loop resolution
On Tue, Nov 23, 2021 at 07:14:04PM +0530, Siddhesh Poyarekar wrote:
> > 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.
I thought you'd in compute_builtin_object_size do something like:
osi.depths = NULL;
osi.stack = NULL;
osi.tos = NULL;
if (object_size_type & OST_DYNAMIC)
{
osi.tempsize_objs.create (0);
// All the new stuff in there
goto done;
}
/* First pass: walk UD chains, compute object sizes that
can be computed. osi.reexamine bitmap at the end will
contain what variables were found in dependency cycles
and therefore need to be reexamined. */
osi.pass = 0;
osi.changed = false;
// Current code
done:
BITMAP_FREE (osi.reexamine);
BITMAP_FREE (osi.visited);
}
or so. The new routine you've added used just for OST_DYNAMIC, keep the
old ones like check_for_plus_in_loops{,_1}, and for the likes of
cond_expr_object_size tweak it so that it can be used in both modes.
Jakub
Re: [PATCH 04/10] tree-object-size: Single pass dependency loop resolution
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
Re: [PATCH 04/10] tree-object-size: Single pass dependency loop resolution
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. > gcc/ChangeLog: > > * tree-object-size.c (struct object_size_info): Remove pass, > changed, depths, stack and tos. Add tempsize_objs. > (OST_TREE_CODE): New macro. > (expr_object_size, merge_object_sizes, plus_stmt_object_size, > cond_expr_object_size): Return tree and don't pass pointer tree. > (object_sizes_set): Return void. Adjust implementation to hold > placeholder SSA names and their values in different slots. > (addr_object_size): Adjust for single pass. > (reducing_size, estimate_size, resolve_dependency_loops): New > functions. > (compute_builtin_object_size): Call them. > (make_tempsize): New function. > (collect_object_sizes_for): Use it. Update object_sizes at most > twice. > (check_for_plus_in_loops, check_for_plus_in_loops_1): Remove > functions. > > gcc/testsuite/ChangeLog: > > * gcc.dg/builtin-object-size-1.c (test6): New test for > passthrough. > * gcc.dg/builtin-object-size-2.c: Likewise. > * gcc.dg/builtin-object-size-3.c: Likewise. > * gcc.dg/builtin-object-size-4.c: Likewise. Jakub
