On Sat, May 16, 2026 at 6:19 PM Richard Sandiford <[email protected]> wrote: > > "H.J. Lu" <[email protected]> writes: > > I put users/hjl/pr124798/master branch at > > > > https://gitlab.com/x86-gcc/gcc/-/commits/users/hjl/pr124798/master > > > > I applied all of your patches and merge some of them into > > > > commit 9d0043fdcbab21ed2cc3ac547fe4b37c6f02ac94 > > Author: H.J. Lu <[email protected]> > > Date: Tue Apr 14 18:37:20 2026 +0800 > > > > x86: Implement TARGET_FNTYPE_ABI > > > > with these changes: > > Thanks. The updates mostly look good to me FWIW. I noticed > after posting that I'd failed to remove the final loop in > ix86_conditional_register_usage, so thanks for catching that. > > On: > > > @@ -21842,15 +21869,32 @@ ix86_initialize_abi (unsigned int abi_id, > > HARD_REG_SET full_reg_clobbers) > > } > > > > /* Return the descriptor of no_callee_saved_registers function type. > > - None of the enabled registers are preserved, except for the common > > - rules applied by ix86_initialize_abi. */ > > + None of the registers are preserved, except for frame register to > > + mitigate PR target/114116. > > The reason I changed this is that I don't think the frame pointer handling > is to mitigate that PR, at least not any more. That PR did not make > callers assume that no_callee_saved_registers callees would preserve > the frame pointer. The fix just made a local decision within the callee > itself. And the fix added TYPE_NO_CALLEE_SAVED_REGISTERS_EXCEPT_BP to > represent that local decision. It's always OK for a callee to save a > register that it doesn't "need" to. :) That can sometimes be useful > as a QoI thing. > > The more relevant PR seems to be PR120840, which established that > callers *can* assume that the frame pointer is preserved (at least > by toolchains that have the fix).
PR120840 is about CPython, which has been fixed by commit 9d5efd46b3329f6b7b0eb2983f317687899caba6 Author: H.J. Lu <[email protected]> Date: Sun May 10 18:36:26 2026 +0800 x86-64: Use R11 for DRAP register in preserve_none functions > preserve_none is now a public, cross-toolchain ABI. The de facto > specification seems to be that it preserves the frame pointer. > That's what PR120840 implemented for GCC and is what LLVM seems > to believe [https://godbolt.org/z/KrGePj3or]. > > Even if GCC wanted to take a different approach to PR114116 in future, > it wouldn't be able to change the frame pointer behaviour without an > ABI break with LLVM and with GCC 16. > > In summary: if we want to remove the frame pointer here, it has to be > a guarantee made by the specification. It can't just be a mitigation. > Removing the frame pointer says to callers that the callee *will* preserve > the frame pointer, no matter which toolchain compiled the callee. If we > can't guarantee that, then we need to do something different. > > > + > > + NB: All registers, including disabled registers, aren't preserved > > + so that ABI_NO_CALLEE_SAVED can be used for all callers and callees, > > + including callers with additional registers enabled by target > > + attribute. Disabled registers won't be caller-saved since they > > + won't be accessed in the caller after > > + > > + commit 3050eeeee4183b9723a1465552b5798a1c21d223 > > + Author: H.J. Lu <[email protected]> > > + Date: Fri May 15 08:01:56 2026 +0800 > > + > > + caller_save_regs: Return the enabled registers > > + > > + */ > > > > static const predefined_function_abi & > > ix86_no_callee_saved_abi (void) > > { > > auto &no_callee_saved_abi = function_abis[ABI_NO_CALLEE_SAVED]; > > if (!no_callee_saved_abi.initialized_p ()) > > - ix86_initialize_abi (ABI_NO_CALLEE_SAVED, accessible_reg_set); > > + { > > + HARD_REG_SET full_reg_clobbers = reg_class_contents[ALL_REGS]; > > + ix86_initialize_abi (ABI_NO_CALLEE_SAVED, full_reg_clobbers); > > + } > > return no_callee_saved_abi; > > } > > I'd gone back and forth on whether accessible_reg_set or > reg_class_contents[ALL_REGS] was stylistically better. But despite > what the comment says, I think the behaviour is the same either way. > In particular, the first thing that ix86_initialize_abi does is: > > /* The general rule is that fixed registers should be marked as > call-clobbered. This includes global registers, inaccessible > registers, the flags register, and the FPSR. > > Handle the exceptions below. */ > full_reg_clobbers |= fixed_reg_set; > > and that would set full_reg_clobbers to ALL_REGS even if we started > with accessible_reg_set. > > Remember that this function_abi is specific to the current target, > and thus to the current accessible_reg_set. (See target-globals.h > and the way that ix86_set_current_function switches targets.) > > That means that we don't have to worry about additional registers. > If additional registers are enabled, we'd switch to a different target, > and that target would have its own version of this ABI, with a different > accessible_reg_set. > > That is, accessible_reg_set and function_abis[ABI_NO_CALLEE_SAVED] > change in tandem. We don't reuse function_abis[ABI_NO_CALLEE_SAVED] > for multiple accessible_reg_sets. > > I agree that reg_class_contents[ALL_REGS] is probably more consistent > with the other functions. But I think the NB is misleading. This is > purely a stylistic choice. > > So my recommendation would be to keep the code change but drop the > comment change. Or perhaps we should change the comment to say > something else, since I suppose this discussion proves that the > situation isn't obvious. :) I have reverted my change and kept yours. I added a test, gcc.target/i386/no-callee-saved-20.c, to convince myself that your change is OK. Please checkout the current branch at https://gitlab.com/x86-gcc/gcc/-/tree/users/hjl/pr124798/master?ref_type=heads with commit e29afe397b3fb54b5487c7a0e042689c47c1f284 Author: H.J. Lu <[email protected]> AuthorDate: Tue Apr 14 18:37:20 2026 +0800 Commit: H.J. Lu <[email protected]> CommitDate: Sat May 16 10:30:28 2026 +0800 x86: Implement TARGET_FNTYPE_ABI to see if it is what you expected. > -- > > There was a typo in one of my patches, sorry. I've attached a patch below. > > -- > > How would you like to handle the submission? Will you submit the x86 > prepatches and the main patch as a single series, to show the prepatches > in context? Or would you prefer me to send the prepatches separately? > Either way's fine with me. I'd like to submit 4 x86 backend patches: e29afe397b3 x86: Implement TARGET_FNTYPE_ABI c253b274314 i386: Avoid always-true condition 9aedbc999b6 i386: Avoid reading call_used_regs e436a833134 i386: Split out call_saved_registers_type detection as a single series after your patch 8a8d266476d Add CALL_INSN_ABI_ID has been merged. Thanks. > Thanks, > Richard > > > diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md > index 19bd8b7cbf3..f195a96359e 100644 > --- a/gcc/config/i386/i386.md > +++ b/gcc/config/i386/i386.md > @@ -518,7 +518,7 @@ > (ABI_DEFAULT 0) > ;; An ABI that describes the effect of vzeroupper. vzeroupper is > ;; represented as a call because calls are the main mechanism for > - ;; expression partial register clobbers. > + ;; expressing partial register clobbers. > (ABI_VZEROUPPER 1) > ;; ABI_ALTERNATE is the Windows function ABI if ix86_abi == SYSV_ABI > ;; and is the SYSV function ABI if ix86_abi == MS_ABI. -- H.J.
