Re: [PATCH] testsuite, X86, Darwin: Fix bf16 ABI tests for Mach-O/macOS ABI.

[Iain Sandoe <iain@sandoe.co.uk>]

Hi Uros,

> On 5 Dec 2022, at 21:07, Uros Bizjak via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> On Mon, Dec 5, 2022 at 3:54 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>> 
>> Hi Uros,
>> 
>>> On 5 Dec 2022, at 10:37, Uros Bizjak via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>>> 
>>> On Sun, Dec 4, 2022 at 9:30 PM Iain Sandoe <iain@sandoe.co.uk> wrote:
>>>> 
>> 
>>>> gcc/testsuite/ChangeLog:
>>>> 
>>>>       * gcc.target/x86_64/abi/bf16/args.h: Make xmm_regs, x87_regs extern.
>>>>       * gcc.target/x86_64/abi/bf16/m256bf16/args.h: Likewise.
>>>>       * gcc.target/x86_64/abi/bf16/m512bf16/args.h: Likewise.
>>>>       * gcc.target/x86_64/abi/bf16/asm-support.S: Add Mach-O variant.
>>>>       * gcc.target/x86_64/abi/bf16/m256bf16/asm-support.S: Likewise.
>>>>       * gcc.target/x86_64/abi/bf16/m512bf16/asm-support.S: Likewise.
>>> 
>>> Please note that in other directories asm-support-darwin.s is
>>> introduced and included via .exp file. Is there a reason a different
>>> approach is introduced here?
>> 
>> Since it seems that testcases get added and amended without considering any
>> sub-target apart from x86_64-linux-gnu (even by very experienced contributors),
>> I was hoping that the Darwin section might prompt folks to remember that there
>> are several other sub-targets.
>> 
>> However, the main thing is to fix the tests .. so here’s a version using separate
>> files.
> 
> extern void (*callthis)(void);
> extern unsigned long long
> rax,rbx,rcx,rdx,rsi,rdi,rsp,rbp,r8,r9,r10,r11,r12,r13,r14,r15;
> -XMM_T xmm_regs[16];
> -X87_T x87_regs[8];
> +extern XMM_T xmm_regs[16];
> +extern X87_T x87_regs[8];
> 
> Do you still need this change? Existing test files are compiled without extern.
> 
> +    .globl    _callthis
> +    .zerofill __DATA,__bss,_callthis,8,3
> +    .globl    _rax
> +    .zerofill __DATA,__bss,_rax,8,3
> +    .globl    _rbx
> +    .zerofill __DATA,__bss,_rbx,8,3
> +    .globl    _rcx
> +    .zerofill __DATA,__bss,_rcx,8,3
> +    .globl    _rdx
> +    .zerofill __DATA,__bss,_rdx,8,3
> ...
> 
> I wonder if the above approach is better than existing:
> 
>    .comm    _callthis,8
>    .comm    _rax,8
>    .comm    _rbx,8
>    .comm    _rcx,8
>    .comm    _rdx,8
> ...

As noted in the changelog, direct access to common data is not permitted in the Darwin
ABI [for x86_64, it would need to be _xxx@GOTPCREL(%rip)..] that’s why these have
been moved to bss.

> It is strange to have two different approaches for similar tests. If
> the new approach is better, we should also change existing asm-support
> files.

could be, I have not checked other case so far (extremely limited time at the moment)

Quite likely, the accesses work in the testcases, despite violating the ABI.

Iain