Re: [Patch 1/2 AArch64/ARM] Give AArch64 ROR (Immediate) a new type attribute

[Kyrill Tkachov <kyrylo.tkachov@arm.com>]

Hi James,

On 25/09/15 08:59, James Greenhalgh wrote:
> Hi,
>
> This patch splits the "shift_imm" type attribute used by AArch64 in
> two - giving rotate_imm and shift_imm.
>
> We then apply this transform across the AArch64 pipeline descriptions
> which have modelling for shift_imm (cortex-a53, cortex-a57, thunderx).
> This should give no functional change to these models.
>
> Bootstrapped and tested on aarch64-none-linux-gnu, and
> arm-none-linux-gnueabihf with no issues.
>
> OK?
>
> Thanks,
> James
>
> ---
> 2015-09-25  James Greenhalgh  <james.greenhalgh@arm.com>
>
> 	* config/arm/types.md (type): Add rotate_imm.
> 	* config/aarch64/aarch64.md (*ror<mode>3_insn): Split out the
> 	ROR immediate case.
> 	(*rorsi3_insn_uxtw): Likewise.
> 	* config/aarch64/thunderx.md (thunderx_shift): Add rotate_imm.
> 	* config/arm/cortex-a53.md (cortex_a53_alu_shift): Add rotate_imm.
> 	* config/arm/cortex-a57.md (cortex_a53_alu): Add rotate_imm.
>

--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -3807,13 +3807,15 @@
  
  ;; Rotate right
  (define_insn "*ror<mode>3_insn"
-  [(set (match_operand:GPI 0 "register_operand" "=r")
-        (rotatert:GPI
-          (match_operand:GPI 1 "register_operand" "r")
-          (match_operand:QI 2 "aarch64_reg_or_shift_imm_<mode>" "rUs<cmode>")))]
+  [(set (match_operand:GPI 0 "register_operand" "=r,r")
+     (rotatert:GPI
+       (match_operand:GPI 1 "register_operand" "r,r")
+       (match_operand:QI 2 "aarch64_reg_or_shift_imm_<mode>" "r,Us<cmode>")))]
    ""
-  "ror\\t%<w>0, %<w>1, %<w>2"
-  [(set_attr "type" "shift_reg")]
+  "@
+   ror\\t%<w>0, %<w>1, %<w>2
+   ror\\t%<w>0, %<w>1, %<w>2"
+  [(set_attr "type" "shift_reg, rotate_imm")]
  )
  
AFAIK since the output template for the two alternatives is identical you don't need to specify multiple
identical output templates using '@'. You can just specify the alternative values for the "type" attribute.
See the *sub_shiftsi pattern in the arm backend for an example of that.

arm-wise this patch is ok since you don't actually introduce usage of the new type to any arm patterns.

Thanks,
Kyrill