aarch64 : Mark rotate immediates with '#' as per DDI0487iFc.

Hi,

The armv8_arm manual [C6.2.226, ROR (immediate)] uses a # in front
of the immediate rotation quantity.

Although, it seems, GAS is able to infer the # (or is leninent about
its absence) assemblers based on the LLVM back end expect it and error out.

tested on aarch64-linux-gnu (gcc115) and aarch64-darwin20 (experimental)

OK for master?
thanks
Iain

gcc/ChangeLog:

	* config/aarch64/aarch64.md (<optab>_rol<mode>3): Add a '#'
	mark in front of the immediate quantity.
	(<optab>_rolsi3_uxtw): Likewise.
---
gcc/config/aarch64/aarch64.md | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

Iain Sandoe <iain@sandoe.co.uk> writes:
> Hi,
>
> The armv8_arm manual [C6.2.226, ROR (immediate)] uses a # in front
> of the immediate rotation quantity.
>
> Although, it seems, GAS is able to infer the # (or is leninent about
> its absence) assemblers based on the LLVM back end expect it and error out.
>
> tested on aarch64-linux-gnu (gcc115) and aarch64-darwin20 (experimental)

Sorry for the slow reply, didn't see this till now.

The patch is OK in principle, and personally I prefer “#”.  But how far
does this spread?  Are only ROR modifiers on logical patterns affected?
Or is the use of a paranthesised expression instead of a literal the thing
that makes the difference?  GCC is generally quite lax at including “#”,
so I'd expect more fallout than this.

Thanks,
Richard

>
> OK for master?
> thanks
> Iain
>
> gcc/ChangeLog:
>
> 	* config/aarch64/aarch64.md (<optab>_rol<mode>3): Add a '#'
> 	mark in front of the immediate quantity.
> 	(<optab>_rolsi3_uxtw): Likewise.
> ---
> gcc/config/aarch64/aarch64.md | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 45d9c6ac45a..e0de82c938a 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -4416,7 +4416,7 @@
> 		      (match_operand:QI 2 "aarch64_shift_imm_<mode>" "n"))
> 		     (match_operand:GPI 3 "register_operand" "r")))]
>   ""
> -  "<logical>\\t%<w>0, %<w>3, %<w>1, ror (<sizen> - %2)"
> +  "<logical>\\t%<w>0, %<w>3, %<w>1, ror #(<sizen> - %2)"
>   [(set_attr "type" "logic_shift_imm")]
> )
>
> @@ -4441,7 +4441,7 @@
> 		      (match_operand:QI 2 "aarch64_shift_imm_si" "n"))
> 		     (match_operand:SI 3 "register_operand" "r"))))]
>   ""
> -  "<logical>\\t%w0, %w3, %w1, ror (32 - %2)"
> +  "<logical>\\t%w0, %w3, %w1, ror #(32 - %2)"
>   [(set_attr "type" "logic_shift_imm")]
> )

Hi Richard,

Richard Sandiford <richard.sandiford@arm.com> wrote:

> Iain Sandoe <iain@sandoe.co.uk> writes:

>> The armv8_arm manual [C6.2.226, ROR (immediate)] uses a # in front
>> of the immediate rotation quantity.
>>
>> Although, it seems, GAS is able to infer the # (or is leninent about
>> its absence) assemblers based on the LLVM back end expect it and error  
>> out.
>>
>> tested on aarch64-linux-gnu (gcc115) and aarch64-darwin20 (experimental)
>
> Sorry for the slow reply, didn't see this till now.

Hmm .. I did CC you directly ( but having some troubles with this ISP which  
I am
trying to resolve - emails going missing or have to be re-sent …  :/ )

> The patch is OK in principle, and personally I prefer “#”.  But how far
> does this spread?  Are only ROR modifiers on logical patterns affected?
> Or is the use of a paranthesised expression instead of a literal the thing
> that makes the difference?

perhaps,

Unfortunately, I have only a cause and effect mechanism to check this (i.e.
problems that prevent bootstrap or ones I’ve triaged in the testsuite fails).

For X86 and PPC I have a more direct way of invoking the LLVM backend (not  
had
time to extend that to aarch64 yet).

.. but via XCode (which is what 99.99% of macOS folks will use as their  
‘binutils’) it’s
done by “as” ==> "clang -cc1as” (effectively an assembler with  
preprocessing).  This
issue certainly fires there - and I’d have no reason to think that the  
preprocessor would
make any material difference, so it’s likely that the LLVM backend is not  
set up to
deduce that the value is a constant and infer the ‘#’.

> GCC is generally quite lax at including “#”, so I'd expect more fallout  
> than this.

This is one of three issues (that are not directly related to mach-o format  
or relocation
syntax) that I’ve encountered in my experimental port that happen to  
trigger in bootstrap.

(the others are a format specifier type naming clash and one related to  
Darwin having
  signed chars by default - so this is the only asm syntax one remaining - Alex fixed the
  other fails earlier in the year).

Of course, it’s possible that there are other cases in the testsuite  
fallout that i’ve not yet
examined - there’s quite a lot still to triage :(.

Once upon a time there were some tests (at least for PPC and X86) that  
essentially
were a single file containing all the insns - so that could be thrown at an  
assembler
and would be expected to complete without error - but I don’t know if  
there’s an
equivalent for aarch64.

cheers
Iain

* I suppose someone could grep ‘#’ in the armv8_arm and c.f. cases against  
aarch64.md
(I dare not volunteer, at present, since I’m already way overcomitted with  
non-$dayjob).

>
> Thanks,
> Richard
>
>> OK for master?
>> thanks
>> Iain
>>
>> gcc/ChangeLog:
>>
>> 	* config/aarch64/aarch64.md (<optab>_rol<mode>3): Add a '#'
>> 	mark in front of the immediate quantity.
>> 	(<optab>_rolsi3_uxtw): Likewise.
>> ---
>> gcc/config/aarch64/aarch64.md | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
>> index 45d9c6ac45a..e0de82c938a 100644
>> --- a/gcc/config/aarch64/aarch64.md
>> +++ b/gcc/config/aarch64/aarch64.md
>> @@ -4416,7 +4416,7 @@
>> 		      (match_operand:QI 2 "aarch64_shift_imm_<mode>" "n"))
>> 		     (match_operand:GPI 3 "register_operand" "r")))]
>>  ""
>> -  "<logical>\\t%<w>0, %<w>3, %<w>1, ror (<sizen> - %2)"
>> +  "<logical>\\t%<w>0, %<w>3, %<w>1, ror #(<sizen> - %2)"
>>  [(set_attr "type" "logic_shift_imm")]
>> )
>>
>> @@ -4441,7 +4441,7 @@
>> 		      (match_operand:QI 2 "aarch64_shift_imm_si" "n"))
>> 		     (match_operand:SI 3 "register_operand" "r"))))]
>>  ""
>> -  "<logical>\\t%w0, %w3, %w1, ror (32 - %2)"
>> +  "<logical>\\t%w0, %w3, %w1, ror #(32 - %2)"
>>  [(set_attr "type" "logic_shift_imm")]
>> )

Iain Sandoe <iain@sandoe.co.uk> writes:
> Hi Richard,
>
> Richard Sandiford <richard.sandiford@arm.com> wrote:
>
>> Iain Sandoe <iain@sandoe.co.uk> writes:
>
>>> The armv8_arm manual [C6.2.226, ROR (immediate)] uses a # in front
>>> of the immediate rotation quantity.
>>>
>>> Although, it seems, GAS is able to infer the # (or is leninent about
>>> its absence) assemblers based on the LLVM back end expect it and error  
>>> out.
>>>
>>> tested on aarch64-linux-gnu (gcc115) and aarch64-darwin20 (experimental)
>>
>> Sorry for the slow reply, didn't see this till now.
>
> Hmm .. I did CC you directly ( but having some troubles with this ISP which  
> I am
> trying to resolve - emails going missing or have to be re-sent …  :/ )

Yeah, I got the message, I just didn't see it, sorry.

>> The patch is OK in principle, and personally I prefer “#”.  But how far
>> does this spread?  Are only ROR modifiers on logical patterns affected?
>> Or is the use of a paranthesised expression instead of a literal the thing
>> that makes the difference?
>
> perhaps,

Trying it out locally, that does seem to be the difference:

	and	x1, x2, x3, ror 1    // OK
	and	x1, x2, x3, ror (1)  // Rejected
	and	x1, x2, x3, ror #(1) // OK

Same for the other modifiers I tried.

That doesn't look intentional, but whether it's intentional obviously
isn't important in this context.  So yeah, the patch is OK.

Thanks,
Richard