On Mon, May 18, 2026 at 12:52 AM Richard Sandiford <[email protected]> wrote: > > "H.J. Lu" <[email protected]> writes: > > 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 > > FWIW, I meant the original fix for the PR, i.e. > https://gcc.gnu.org/cgit/gcc/commit/?id=9a602ce3f4c95648c0c48d3f26fc52f69d012505 > > 2025-06-30 H.J. Lu <[email protected]> > > PR target/120840 > * config/i386/i386-expand.cc (ix86_expand_call): Don't mark > hard frame pointer as clobber. > * config/i386/i386-options.cc (ix86_set_func_type): Use > TYPE_NO_CALLEE_SAVED_REGISTERS instead of > TYPE_NO_CALLEE_SAVED_REGISTERS_EXCEPT_BP. > * config/i386/i386.cc (ix86_function_ok_for_sibcall): Remove the > TYPE_NO_CALLEE_SAVED_REGISTERS_EXCEPT_BP check. > (ix86_save_reg): Merge TYPE_NO_CALLEE_SAVED_REGISTERS and > TYPE_PRESERVE_NONE with TYPE_NO_CALLEE_SAVED_REGISTERS_EXCEPT_BP. > * config/i386/i386.h (call_saved_registers_type): Remove > TYPE_NO_CALLEE_SAVED_REGISTERS_EXCEPT_BP. > * doc/extend.texi: Update no_callee_saved_registers documentation.
I think we should preserve frame pointer for now. > > [...] > > 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. > > Looks good to me, thanks. > > >> -- > >> > >> 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. > > Sounds good! Thanks HJ. > > I realise it's a separate PR, but FWIW, on: > > commit 7f45b3dacb97a9880cc8b7332091e174220deadf (HEAD, > tmp/users/hjl/pr124798/master) > Author: H.J. Lu <[email protected]> > Date: Wed May 13 08:27:50 2026 +0800 > > x86-64: Disallow mixed ABI attributes > > In 64-bit mode, since preserve_none, no_callee_saved_registers, ms_abi > and sysv_abi attributes change the calling convention, disallow ms_abi > and sysv_abi attributes with preserve_none and no_callee_saved_registers > attributes if they change the calling convention. > > gcc/ > > PR target/125297 > * config/i386/i386.cc (ix86_type_no_callee_saved_registers_p): > Add an argument to return the attribute string. > (ix86_function_type_abi): In 64-bit mode, disallow ms_abi and > sysv_abi attributes with preserve_none and > no_callee_saved_registers attributes if they change the calling > convention. > > I'm not sure that ix86_function_type_abi is the place to do this. > ix86_function_type_abi is supposed to be a pure query function, > with no side effects. There is a different approach for PR target/125297: https://patchwork.sourceware.org/project/gcc/patch/[email protected]/ It ignores MS_ABI when preserve_none is used. It looks reasonable except for some test issues. > The incompatibility is between one type attribute and another > type attribute, so it feels like something that could/should be > reported when the second attribute is added. > > Thanks, > Richard -- H.J.
