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

[James Greenhalgh <james dot greenhalgh at arm dot com>]

On Fri, Sep 25, 2015 at 01:53:35PM +0100, Kyrill Tkachov wrote:
> 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?
> >
> --- 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 for the review and the tip. Patch updated and attached after
checking that the compiler still builds, and that we still schedule
appropriately.

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.

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 78b9ae2..090ad65 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -3807,13 +3807,13 @@
 
 ;; 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")]
+  [(set_attr "type" "shift_reg, rotate_imm")]
 )
 
 ;; zero_extend version of above
@@ -3902,7 +3902,7 @@
   operands[3] = GEN_INT (<sizen> - UINTVAL (operands[2]));
   return "ror\\t%<w>0, %<w>1, %3";
 }
-  [(set_attr "type" "shift_imm")]
+  [(set_attr "type" "rotate_imm")]
 )
 
 ;; zero_extend version of the above
@@ -3916,7 +3916,7 @@
   operands[3] = GEN_INT (32 - UINTVAL (operands[2]));
   return "ror\\t%w0, %w1, %3";
 }
-  [(set_attr "type" "shift_imm")]
+  [(set_attr "type" "rotate_imm")]
 )
 
 (define_insn "*<ANY_EXTEND:optab><GPI:mode>_ashl<SHORT:mode>"
diff --git a/gcc/config/aarch64/thunderx.md b/gcc/config/aarch64/thunderx.md
index cf96368..3dae963 100644
--- a/gcc/config/aarch64/thunderx.md
+++ b/gcc/config/aarch64/thunderx.md
@@ -39,7 +39,7 @@
 
 (define_insn_reservation "thunderx_shift" 1
   (and (eq_attr "tune" "thunderx")
-       (eq_attr "type" "bfm,extend,shift_imm,shift_reg,rbit,rev"))
+       (eq_attr "type" "bfm,extend,rotate_imm,shift_imm,shift_reg,rbit,rev"))
   "thunderx_pipe0 | thunderx_pipe1")
 
 
diff --git a/gcc/config/arm/cortex-a53.md b/gcc/config/arm/cortex-a53.md
index db572f6..3fa0625 100644
--- a/gcc/config/arm/cortex-a53.md
+++ b/gcc/config/arm/cortex-a53.md
@@ -76,7 +76,7 @@
                         alu_sreg,alus_sreg,logic_reg,logics_reg,\
                         adc_imm,adcs_imm,adc_reg,adcs_reg,\
                         adr,bfm,csel,clz,rbit,rev,alu_dsp_reg,\
-                        shift_imm,shift_reg,\
+                        rotate_imm,shift_imm,shift_reg,\
                         mov_imm,mov_reg,mvn_imm,mvn_reg,\
                         mrs,multiple,no_insn"))
   "cortex_a53_slot_any")
diff --git a/gcc/config/arm/cortex-a57.md b/gcc/config/arm/cortex-a57.md
index a32c848..d6ce440 100644
--- a/gcc/config/arm/cortex-a57.md
+++ b/gcc/config/arm/cortex-a57.md
@@ -296,7 +296,7 @@
 			alu_sreg,alus_sreg,logic_reg,logics_reg,\
 			adc_imm,adcs_imm,adc_reg,adcs_reg,\
 			adr,bfm,clz,rbit,rev,alu_dsp_reg,\
-			shift_imm,shift_reg,\
+			rotate_imm,shift_imm,shift_reg,\
 			mov_imm,mov_reg,\
 			mvn_imm,mvn_reg,\
 			mrs,multiple,no_insn"))
diff --git a/gcc/config/arm/types.md b/gcc/config/arm/types.md
index ec609ae..534be74 100644
--- a/gcc/config/arm/types.md
+++ b/gcc/config/arm/types.md
@@ -120,6 +120,7 @@
 ;                    final output, thus having no impact on scheduling.
 ; rbit               reverse bits.
 ; rev                reverse bytes.
+; rotate_imm         rotate by immediate.
 ; sdiv               signed division.
 ; shift_imm          simple shift operation (LSL, LSR, ASR, ROR) with an
 ;                    immediate.
@@ -627,6 +628,7 @@
   nop,\
   rbit,\
   rev,\
+  rotate_imm,\
   sdiv,\
   shift_imm,\
   shift_reg,\