Hi,
On Tue, Jul 13 2021, Alexandre Oliva wrote:
> I'm working on a feature that involves creating wrappers for some
> functions, using alternate means to pass variable argument lists to
> the wrapped versions. The split-out (wrapped) function shouldn't be
> stdarg, and though comments in m_always_copy_start in
> ipa_param_adjustments suggested that making it negative would cause
> variable arguments to be dropped, this was not the case, and AFAICT
> there was no way to accomplish that.
>
> Perhaps there was no such intent implied by the comments, but that
> sounded like a reasonable plan to me, so I went ahead an implemented
> it. With this patch, a negative m_always_copy_start drops the
> variable argument list marker from a cloned function's type, and the
> stdarg flag from the cloned cfun data structure.
>
It probably was my general intent, at least the comments really suggest
so :-)
However, my intent also was that functions which should have all their
arguments copied as they are (and only the return value dropped) should
be marked with NULL m_adj_params and m_always_copy_start set to zero.
Unfortunately, I did not make that work in time, I only vaguely remember
that ipa-split was somehow problematic. I decided that I would not be
making a huge patch even bigger and settled on the idea that, at least
temporarily, m_always_copy_start would always be the initial number of
parameters.
As a consequence I also got lazy and used that assumption in
ipa_param_adjustments::modify_call where m_always_copy_start is meant to
be the original number of PARM_DECLs for which we may attempt to
generate debug info in:
for (tree old_parm = DECL_ARGUMENTS (old_decl);
old_parm && i < old_nargs && ((int) i) < m_always_copy_start;
old_parm = DECL_CHAIN (old_parm), i++)
Fortunately, my recent re-write made that count available through other
means and we can now just use the orig_nargs local variable which is
already correctly computed at the beginning of the function.
> Regstrapped on x86_64-linux-gnu. Ok to install?
Generally speaking, I agree that relaxing this assumption in general is
a good idea and so the patch is OK with this one additional fix
described above (I did not bootstrap and test it myself but it should
work (TM)).
Note that there are asserts in that check that m_always_copy_start is
either negative or the initial count of arguments so even after this
change m_always_copy_start cannot have an arbitrary value (and with the
exception of allowing zero I think that is a reasonable limitation).
>
> for gcc/ChangeLog
>
> * ipa-param-manipulation.c (build_adjusted_function_type): Add
> skip_valist, obey it.
> (ipa_param_adjustments::build_new_function_type): Take
> negative m_always_copy_start as skip_valist.
> (ipa_param_body_adjustmnets::modify_formal_parameters):
> Likewise.
> * tee-inline.c (initialize_cfun): Clear stdarg if function
> type doesn't support it.
tRee-inline.c.
Thanks,
Martin
