On Thu, 3 Oct 2024, Jason Merrill wrote:
> On 10/3/24 12:38 PM, Jason Merrill wrote:
> > On 10/2/24 7:50 AM, Richard Biener wrote:
> > > This reduces peak memory usage by 20% for a specific testcase.
> > >
> > > Bootstrapped and tested on x86_64-unknown-linux-gnu.
> > >
> > > It's very ugly so I'd appreciate suggestions on how to handle such
> > > situations better?
> >
> > I'm pushing this alternative patch, tested x86_64-pc-linux-gnu.
>
> OK, apparently that was both too clever and not clever enough. Replacing it
> with this one that's much closer to yours.
>
> Jason
> From: Jason Merrill <ja...@redhat.com>
> Date: Thu, 3 Oct 2024 16:31:00 -0400
> Subject: [PATCH] c++: free garbage vec in coerce_template_parms
> To: gcc-patches@gcc.gnu.org
>
> coerce_template_parms can create two different vecs for the inner template
> arguments, new_inner_args and (potentially) the result of
> expand_template_argument_pack. One or the other, or possibly both, end up
> being garbage: in the typical case, the expanded vec is garbage because it's
> only used as the source for convert_template_argument. In some dependent
> cases, the new vec is garbage because we decide to return the original args
> instead. In these cases, ggc_free the garbage vec to reduce the memory
> overhead of overload resolution.
>
> gcc/cp/ChangeLog:
>
> * pt.cc (coerce_template_parms): Free garbage vecs.
>
> Co-authored-by: Richard Biener <rguent...@suse.de>
> ---
> gcc/cp/pt.cc | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 20affcd65a2..4ceae1d38de 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -9275,6 +9275,7 @@ coerce_template_parms (tree parms,
> {
> /* We don't know how many args we have yet, just use the
> unconverted (and still packed) ones for now. */
> + ggc_free (new_inner_args);
> new_inner_args = orig_inner_args;
> arg_idx = nargs;
> break;
> @@ -9329,7 +9330,8 @@ coerce_template_parms (tree parms,
> = make_pack_expansion (conv, complain);
>
> /* We don't know how many args we have yet, just
> - use the unconverted ones for now. */
> + use the unconverted (but unpacked) ones for now. */
> + ggc_free (new_inner_args);
I'm a bit worried about these ggc_frees. If an earlier template
parameter is a constrained auto NTTP then new_inner_args/new_args could
have been captured by the satisfaction cache during coercion for that
argument, and so we'd be freeing a vector that's still live?
> new_inner_args = inner_args;
> arg_idx = nargs;
> break;
> @@ -9442,6 +9444,12 @@ coerce_template_parms (tree parms,
> SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (new_inner_args,
> TREE_VEC_LENGTH (new_inner_args));
>
> + /* If we expanded packs in inner_args and aren't returning it now, the
> + expanded vec is garbage. */
> + if (inner_args != new_inner_args
> + && inner_args != orig_inner_args)
> + ggc_free (inner_args);
> +
> return return_full_args ? new_args : new_inner_args;
> }
>
> --
> 2.46.2
>
