Re: [PATCH v2] rs6000: Stackoverflow in optimized code on PPC [PR100799]

["Kewen.Lin" <linkw@linux.ibm.com>]

Hi Jakub,

on 2024/4/3 16:35, Jakub Jelinek wrote:
> On Wed, Apr 03, 2024 at 01:18:54PM +0800, Kewen.Lin wrote:
>>> I'd prefer not to remove DECL_ARGUMENTS chains, they are valid arguments that just some
>>> invalid code doesn't pass.  By removing them you basically always create an
>>> invalid case, this time in the other direction, valid caller passes more
>>> arguments than the callee (invalidly) expects.
>>
>> Thanks for the comments, do you mean it can affect the arguments validation when there
>> is explicit function declaration with interface?  Then can we strip them when we are
>> going to expand them (like checking currently_expanding_function_start)?
> 
> I'd prefer not stripping them at all; they are clearly marked as perhaps not
> passed in buggy programs (the DECL_HIDDEN_STRING_LENGTH argument) and
> removing them implies the decl is a throw away, that after expansion

Yes, IMHO it's safe as they are unused.

> nothing will actually look at it anymore.  I believe that is the case of
> function bodies, we expand them into RTL and throw away the GIMPLE, and
> after final clear the bodies, but it is not the case of the FUNCTION_DECL
> or its DECL_ARGUMENTs etc.  E.g. GIMPLE optimizations or expansion of
> callers could be looking at those as well.

At expand time GIMPLE optimizations should already finish, so it should be
safe to strip them at that time?  It would surprise me if expansions of
callers will look at callee's information, it's more like what should be
done in IPA analysis instead?

> 
>> since from the
>> perspective of resulted assembly, with this workaround, the callee can:
>>   1) pass the hidden args in unexpected GPR like r11, ... at -O0;
>>   2) get rid of such hidden args as they are unused at -O2;
>> This proposal aims to make the assembly at -O0 not to pass with r11... (same as -O2),
>> comparing to the assembly at O2, the mismatch isn't actually changed.
> 
> The aim for the workaround was just avoid assuming there is a argument save
> area in the caller stack when it is sometimes missing.

Yeah, understood.

> If you are looking for optimizations where nothing actually passes the
> unneeded arguments and nothing expects them to be passed, then it is a task
> for IPA optimizations and should be done solely if IPA determines that all
> callers can be adjusted together with the callee; I think IPA already does
> that in that case for years, regardless if it is DECL_HIDDEN_STRING_LENGTH
> PARM_DECL or not.

No, it's not what I was looking for.  Peter's comments made me feel it's not
good to have assembly at O0 like:

        std %r3,112(%r31)
        std %r4,120(%r31)
        std %r5,128(%r31)
        std %r6,136(%r31)
        std %r7,144(%r31)
        std %r8,152(%r31)
        std %r9,160(%r31)
        std %r10,168(%r31)
        std %r11,176(%r31) // this mislead people that we pass 9th arg via r11,
                           // it would be nice not to have it.

so I was thinking if there is some way to get rid of it.

BR,
Kewen