diff mbox

[AArch64] Use cinc mnemonic for *csinc2<mode>_insn

Message ID 55A663C8.4070201@arm.com
State New
Headers show

Commit Message

Kyrylo Tkachov July 15, 2015, 1:44 p.m. UTC
Hi all,
This pattern performs a csinc of the same register in both operands.
This form can be written using the shorter alias CINC.
The ARMv8-A ARM says:

"CINC <Wd>, <Wn>, <cond>
is equivalent to
CSINC <Wd>, <Wn>, <Wn>, invert(<cond>)"

So the patch switches the condition output modifier from 'M' to the inverse 'm'.

I don't think we emit cinc anywhere else in aarch64.md so this will exercise the assembler
a tiny bit more (no fallout detected in my testing) and make the output a bit more concise.
Again, this is an alias, not a different instruction, so there are no performance/behaviour implications

Bootstrapped and tested on aarch64.

Ok for trunk?

Thanks,
Kyrill

2015-07-15  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * config/aarch64/aarch64.md (*csinc2<mode>_insn): Use cinc mnemonic.

Comments

James Greenhalgh July 15, 2015, 2 p.m. UTC | #1
On Wed, Jul 15, 2015 at 02:44:40PM +0100, Kyrill Tkachov wrote:
> Hi all,
> This pattern performs a csinc of the same register in both operands.
> This form can be written using the shorter alias CINC.
> The ARMv8-A ARM says:
> 
> "CINC <Wd>, <Wn>, <cond>
> is equivalent to
> CSINC <Wd>, <Wn>, <Wn>, invert(<cond>)"
> 
> So the patch switches the condition output modifier from 'M' to the inverse 'm'.
> 
> I don't think we emit cinc anywhere else in aarch64.md so this will exercise
> the assembler a tiny bit more (no fallout detected in my testing) and make
> the output a bit more concise.

More importantly this puts us in line with the preferred disassembly, which
I think is a desirable position. If we can ever get here using the
zero register you should really put out cset, but having different
alternatives in this pattern to cover the different aliasing conditions
does seem overkill!

> Again, this is an alias, not a different instruction, so there are no
> performance/behaviour implications
> 
> Bootstrapped and tested on aarch64.
> 
> Ok for trunk?

Yup, OK. Watch out for the mangling of the tab in your proposed ChangeLog.

Thanks,
James

> 
> Thanks,
> Kyrill
> 
> 2015-07-15  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>      * config/aarch64/aarch64.md (*csinc2<mode>_insn): Use cinc mnemonic.

> commit ec05547074dd575471ebafdd10975f438e3361f6
> Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
> Date:   Wed Jul 8 11:04:47 2015 +0100
> 
>     [AArch64] Use cinc mnemonic for *csinc2<mode>_insn
> 
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 300537e..57a3360 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -3036,7 +3036,7 @@ (define_insn "*csinc2<mode>_insn"
>          (plus:GPI (match_operand 2 "aarch64_comparison_operation" "")
>                    (match_operand:GPI 1 "register_operand" "r")))]
>    ""
> -  "csinc\\t%<w>0, %<w>1, %<w>1, %M2"
> +  "cinc\\t%<w>0, %<w>1, %m2"
>    [(set_attr "type" "csel")]
>  )
>
Kyrylo Tkachov July 15, 2015, 2:12 p.m. UTC | #2
On 15/07/15 15:00, James Greenhalgh wrote:
> On Wed, Jul 15, 2015 at 02:44:40PM +0100, Kyrill Tkachov wrote:
>> Hi all,
>> This pattern performs a csinc of the same register in both operands.
>> This form can be written using the shorter alias CINC.
>> The ARMv8-A ARM says:
>>
>> "CINC <Wd>, <Wn>, <cond>
>> is equivalent to
>> CSINC <Wd>, <Wn>, <Wn>, invert(<cond>)"
>>
>> So the patch switches the condition output modifier from 'M' to the inverse 'm'.
>>
>> I don't think we emit cinc anywhere else in aarch64.md so this will exercise
>> the assembler a tiny bit more (no fallout detected in my testing) and make
>> the output a bit more concise.
> More importantly this puts us in line with the preferred disassembly, which
> I think is a desirable position. If we can ever get here using the
> zero register you should really put out cset, but having different
> alternatives in this pattern to cover the different aliasing conditions
> does seem overkill!

The predicate here is a register_operand so a (const_int 0)
matching the zero register would never go into this pattern, I think.

I would like it if we output the compact forms in the csneg3, csinv3, csinc3
patterns whenever the register numbers match up, but it would require making
the output templates more verbose, which could hurt the readability of the patterns.


>
>> Again, this is an alias, not a different instruction, so there are no
>> performance/behaviour implications
>>
>> Bootstrapped and tested on aarch64.
>>
>> Ok for trunk?
> Yup, OK. Watch out for the mangling of the tab in your proposed ChangeLog.

Thanks, committed with r225830.

Kyrill

>
> Thanks,
> James
>
>> Thanks,
>> Kyrill
>>
>> 2015-07-15  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>       * config/aarch64/aarch64.md (*csinc2<mode>_insn): Use cinc mnemonic.
>> commit ec05547074dd575471ebafdd10975f438e3361f6
>> Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
>> Date:   Wed Jul 8 11:04:47 2015 +0100
>>
>>      [AArch64] Use cinc mnemonic for *csinc2<mode>_insn
>>
>> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
>> index 300537e..57a3360 100644
>> --- a/gcc/config/aarch64/aarch64.md
>> +++ b/gcc/config/aarch64/aarch64.md
>> @@ -3036,7 +3036,7 @@ (define_insn "*csinc2<mode>_insn"
>>           (plus:GPI (match_operand 2 "aarch64_comparison_operation" "")
>>                     (match_operand:GPI 1 "register_operand" "r")))]
>>     ""
>> -  "csinc\\t%<w>0, %<w>1, %<w>1, %M2"
>> +  "cinc\\t%<w>0, %<w>1, %m2"
>>     [(set_attr "type" "csel")]
>>   )
>>
diff mbox

Patch

commit ec05547074dd575471ebafdd10975f438e3361f6
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed Jul 8 11:04:47 2015 +0100

    [AArch64] Use cinc mnemonic for *csinc2<mode>_insn

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 300537e..57a3360 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -3036,7 +3036,7 @@  (define_insn "*csinc2<mode>_insn"
         (plus:GPI (match_operand 2 "aarch64_comparison_operation" "")
                   (match_operand:GPI 1 "register_operand" "r")))]
   ""
-  "csinc\\t%<w>0, %<w>1, %<w>1, %M2"
+  "cinc\\t%<w>0, %<w>1, %m2"
   [(set_attr "type" "csel")]
 )