Hi! on 2024/4/3 17:23, Jakub Jelinek wrote: > On Wed, Apr 03, 2024 at 05:02:40PM +0800, Kewen.Lin wrote: >> 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. > > But they are still passed in the usual case. > >>> 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? > > No. > The IPA/post IPA behavior is that IPA optimizations are performed and then > cgraph finalizes one function at a time, going there from modifications > needed from IPA passes, post IPA GIMPLE optimizations, expansion to RTL, > RTL optimizations, emitting assembly, throwing away the body, then picking > another function and repeating that etc. > So, when one function makes it to expansion, if you modify its > DECL_ARGUMENTS etc., all the post IPA GIMPLE optimization passes of other > functions might still see such changes.
Thanks for explaining, I agree it's risky from this perspective. > >> 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? > > Depends on what exactly it is. E.g. C non-prototyped functions have > just DECL_ARGUMENTS to check how many arguments the call should have vs. > what is actually passed. OK. > >> 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. > > You want to optimize at -O0? Don't. I don't really want optimization but try to get rid of the unreasonable assembly code. :) > That will screw up debugging. The function does have that argument, it > should show up in debug info; it should show up also at -O2 in debug info > etc. If you remove chains from DECL_ARGUMENTS, because we have early dwarf > these days, DW_TAG_formal_parameter nodes should have been already created, > but it would mean that DW_AT_location for those arguments likely isn't > filled. Now, for -O2 it might be the case that the argument has useful > location only at the start of the function, could have > DW_OP_entry_value(%r11) afterwards, but at -O0 it better should have some > stack slot into which the argument is saved and DW_AT_location should be > that stack slot. All that should change with the workaround is that if the > stack slot would be normally in the argument save area in the caller's > frame, if such argument save area can't be counted on, then it needs to be > saved in some other stack slot, like arguments are saved to when there are > only <= 8 arguments. Thanks for the details on debugging support, but IIUC with this workaround being adopted, the debuggability on hidden args are already broken, aren't? Since with a usual caller, the actual argument is passed in argument save area, but the callee debug information says the location is %r11 or some other stack slot. I think the difficulty is that: with this workaround, for some arguments we are lying they are not passed in argument save area, then we have to pretend they are passed in r11,r12..., but in fact these registers are not valid to pass arguments, so it's unreasonable and confusing. With your explanation, I agree that stripping DECL_ARGUMENTS chains isn't a good way to eliminate this confusion, maybe always using GP_ARG_MIN_REG/GP_ARG_MAX_REG for things exceeding GP_ARG_MAX_REG can reduce the unreasonableness (but still confusing IMHO). > > Now, sure, if IPA optimizes a call because it can prove it sees a callee and > all its callers and can modify them all, it can optimize away passing of > that argument but 1) it isn't done at -O0/-Og 2) it ensures debug info > reflects the case that the argument isn't passed and arranges for the > callers to provide DW_OP_GNU_parameter_ref/call site info value 3) it > doesn't modify the FUNCTION_DECL itself or its DECL_ARGUMENTS, but makes > a clone of the FUNCTION_DECL and modifies the clones DECL_ARGUMENTS Yes, but it doesn't help our case. BR, Kewen