"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 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.

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

Reply via email to