Arm assembler crc issue

Arm assembler crc issue

[Iain Sandoe]

Hi Richard,


I am being bitten by a problem that falls out from the code that emits

	.arch Armv8.n-a+crc

when the arch is less than Armv8-r.
The code that does this,  in gcc/common/config/aarch64 is quite recent (2022-09).

------

(I admit the permutations are complex and I might have miss-analyzed) - but it appears that llvm assembler (for mach-o, at least) sees an explict mention of an attribute for a feature which is mandatory at a specified arch level as demoting that arch to the minimum that made the explicit feature mandatory.  Of course, it could just be a bug in the handling of transitive feature enables...

the problem is that, for example:

	.arch Armv8.4-a+crc

no longer recognises fp16 insns. (and appending +fp16 does not fix this).

====

Even if upstream LLVM is deemed to be buggy (it does not do what I would expect, at least), and fixed - I will still have a bunch of assembler versions that are broken (before the fix percolates through to downstream xcode) - and the LLVM assembler is the only current option for Darwin.

So, it seems that this ought to be a reasonable configure test:

	.arch armv8.2-a
	.text
m:
	crc32b w0, w1, w2 

and then emit HAS_GAS_AARCH64_CRC_BUG (for example) if that fails to assemble which can be used to make the +crc emit conditional on a broken assembler.

- I am asking here before constructing the patch, in case there’s some reason that doing this at configure time is not acceptable.

thanks
Iain

Re: Arm assembler crc issue

[Iain Sandoe]

correction ...

> On 19 Oct 2023, at 17:41, Iain Sandoe <iain@sandoe.co.uk> wrote:
> 
> Hi Richard,
> 
> 
> I am being bitten by a problem that falls out from the code that emits
> 
> 	.arch Armv8.n-a+crc
> 
> when the arch is less than Armv8-r.
> The code that does this,  in gcc/common/config/aarch64 is quite recent (2022-09).
> 
> ------
> 
> (I admit the permutations are complex and I might have miss-analyzed) - but it appears that llvm assembler (for mach-o, at least) sees an explict mention of an attribute for a feature which is mandatory at a specified arch level as demoting that arch to the minimum that made the explicit feature mandatory.  Of course, it could just be a bug in the handling of transitive feature enables...
> 
> the problem is that, for example:
> 
> 	.arch Armv8.4-a+crc

We are trying to set suitable arch values for apple-a12, apple-m1 and apple-m2;

apple-m1 (as far as i can determine) is   armv8.4-a+fp16+sb+ssbs.

However, the code mentioned emits this as:
	armv8.4-a+crc+fp16+sb+ssbs.

and the LLVM assembler no longer recognises fp16 insns. (the appended +fp16 does not work).

> ====
> 
> Even if upstream LLVM is deemed to be buggy (it does not do what I would expect, at least), and fixed - I will still have a bunch of assembler versions that are broken (before the fix percolates through to downstream xcode) - and the LLVM assembler is the only current option for Darwin.
> 
> So, it seems that this ought to be a reasonable configure test:
> 
> 	.arch armv8.2-a
> 	.text
> m:
> 	crc32b w0, w1, w2 
> 
> and then emit HAS_GAS_AARCH64_CRC_BUG (for example) if that fails to assemble which can be used to make the +crc emit conditional on a broken assembler.
> 
> - I am asking here before constructing the patch, in case there’s some reason that doing this at configure time is not acceptable.
> 
> thanks
> Iain
> 

Re: Arm assembler crc issue

[Richard Sandiford]

Iain Sandoe <iain@sandoe.co.uk> writes:
> Hi Richard,
>
>
> I am being bitten by a problem that falls out from the code that emits
>
> 	.arch Armv8.n-a+crc
>
> when the arch is less than Armv8-r.
> The code that does this,  in gcc/common/config/aarch64 is quite recent (2022-09).

Heh.  A workaround for one assembler bug triggers another assembler bug.

The special treatment of CRC is much older than 2022-09 though.  I think
it dates back to 04a99ebecee885e42e56b6e0c832570e2a91c196 (2016-04),
with 4ca82fc9f86fc1187ee112e3a637cb3ca5d2ef2a providing the more
complete explanation.

>
> ------
>
> (I admit the permutations are complex and I might have miss-analyzed) - but it appears that llvm assembler (for mach-o, at least) sees an explict mention of an attribute for a feature which is mandatory at a specified arch level as demoting that arch to the minimum that made the explicit feature mandatory.  Of course, it could just be a bug in the handling of transitive feature enables...
>
> the problem is that, for example:
>
> 	.arch Armv8.4-a+crc
>
> no longer recognises fp16 insns. (and appending +fp16 does not fix this).
>
> ====
>
> Even if upstream LLVM is deemed to be buggy (it does not do what I would expect, at least), and fixed - I will still have a bunch of assembler versions that are broken (before the fix percolates through to downstream xcode) - and the LLVM assembler is the only current option for Darwin.
>
> So, it seems that this ought to be a reasonable configure test:
>
> 	.arch armv8.2-a
> 	.text
> m:
> 	crc32b w0, w1, w2 
>
> and then emit HAS_GAS_AARCH64_CRC_BUG (for example) if that fails to assemble which can be used to make the +crc emit conditional on a broken assembler.

AIUI the problem was in the CPU descriptions, so I don't think this
would test for the old gas bug that is being worked around.

Perhaps instead we could have a configure test for the bug that you've
found, and disable the crc workaround if so?

Thanks,
Richard

>
> - I am asking here before constructing the patch, in case there’s some reason that doing this at configure time is not acceptable.
>
> thanks
> Iain

Re: Arm assembler crc issue

[Iain Sandoe]

Hi Richard,

> On 19 Oct 2023, at 22:49, Richard Sandiford <richard.sandiford@arm.com> wrote:
> Iain Sandoe <iain@sandoe.co.uk> writes:

>> I am being bitten by a problem that falls out from the code that emits
>> 
>> 	.arch Armv8.n-a+crc
>> 
>> when the arch is less than Armv8-r.
>> The code that does this,  in gcc/common/config/aarch64 is quite recent (2022-09).
> 
> Heh.  A workaround for one assembler bug triggers another assembler bug.

Indeed …  the good news is that the LLVM bug seems fixed on current release (17) and main.
The bad news is that it will likely take some to percolate through (and it won’t help released
toolsets anyway).

<snip>

>> So, it seems that this ought to be a reasonable configure test:
>> 
>> 	.arch armv8.2-a
>> 	.text
>> m:
>> 	crc32b w0, w1, w2 
>> 
>> and then emit HAS_GAS_AARCH64_CRC_BUG (for example) if that fails to assemble which can be used to make the +crc emit conditional on a broken assembler.
> 
> AIUI the problem was in the CPU descriptions, so I don't think this
> would test for the old gas bug that is being worked around.

I see,

> Perhaps instead we could have a configure test for the bug that you've
> found, and disable the crc workaround if so?

OK - I’ll work in that direction, thanks
Iain

> 
> Thanks,
> Richard
> 
>> 
>> - I am asking here before constructing the patch, in case there’s some reason that doing this at configure time is not acceptable.
>> 
>> thanks
>> Iain