Re: [PATCH] AArch64: Improve rotate patterns
Wilco Dijkstra writes:
> Improve and generalize rotate patterns. Rotates by more than half the
> bitwidth of a register are canonicalized to rotate left. Many existing
> shift patterns don't handle this case correctly, so add rotate left to
> the shift iterator and convert rotate left into ror during assembly
> output. Add missing zero_extend patterns for shifted BIC, ORN and EON.
>
> Passes bootstrap and regress. OK for commit?
OK, thanks. I agree it should go in during stage 3 since handling
rotatert by a constant without rotate by a constant is a bug,
like you say. And the port is supposed to be complete wrt.
zero_extend patterns.
FTR, the is_rotl isn't strictly necessary, since we could I think do:
if ( == ROTATE)
But the fact that ROTATE is the left shift isn't very mnemonic,
so I agree the version in the patch is better.
Richard
>
> ChangeLog:
> 2021-12-07 Wilco Dijkstra
>
> * config/aarch64/aarch64.md
> (and_3_compare0): Support rotate left.
> (and_si3_compare0_uxtw): Likewise.
> (_3): Likewise.
> (_si3_uxtw): Likewise.
> (one_cmpl_2): Likewise.
> (_one_cmpl_3): Likewise.
> (_one_cmpl_sidi_uxtw): New pattern.
> (eor_one_cmpl_3_alt): Support rotate left.
> (eor_one_cmpl_sidi3_alt_ze): Likewise.
> (and_one_cmpl_3_compare0): Likewise.
> (and_one_cmpl_si3_compare0_uxtw): Likewise.
> (and_one_cmpl_3_compare0_no_reuse): Likewise.
> (and_3nr_compare0): Likewise.
> (*si3_insn_uxtw): Use SHIFT_no_rotate.
> (rolsi3_insn_uxtw): New pattern.
> * config/aarch64/iterators.md (SHIFT): Add rotate left.
> (SHIFT_no_rotate): Add new iterator.
> (SHIFT:shift): Print rotate left as ror.
> (is_rotl): Add test for left rotate.
>
> * gcc.target/aarch64/ror_2.c: New test.
> * gcc.target/aarch64/ror_3.c: New test.
>
> ---
>
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index
> 5297b2d3f95744ac72e36814c6676cc97478d48b..f80679bcb34ea07918b1a0304b32be436568095d
> 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -4419,7 +4419,11 @@ (define_insn "*and_3_compare0"
> (set (match_operand:GPI 0 "register_operand" "=r")
> (and:GPI (SHIFT:GPI (match_dup 1) (match_dup 2)) (match_dup 3)))]
>""
> - "ands\\t%0, %3, %1, %2"
> +{
> + if ()
> +operands[2] = GEN_INT ( - UINTVAL (operands[2]));
> + return "ands\\t%0, %3, %1, %2";
> +}
>[(set_attr "type" "logics_shift_imm")]
> )
>
> @@ -4436,7 +4440,11 @@ (define_insn "*and_si3_compare0_uxtw"
> (zero_extend:DI (and:SI (SHIFT:SI (match_dup 1) (match_dup 2))
> (match_dup 3]
>""
> - "ands\\t%w0, %w3, %w1, %2"
> +{
> + if ()
> +operands[2] = GEN_INT (32 - UINTVAL (operands[2]));
> + return "ands\\t%w0, %w3, %w1, %2";
> +}
>[(set_attr "type" "logics_shift_imm")]
> )
>
> @@ -4447,7 +4455,11 @@ (define_insn "*_3"
> (match_operand:QI 2 "aarch64_shift_imm_" "n"))
> (match_operand:GPI 3 "register_operand" "r")))]
>""
> - "\\t%0, %3, %1, %2"
> +{
> + if ()
> +operands[2] = GEN_INT ( - UINTVAL (operands[2]));
> + return "\\t%0, %3, %1, %2";
> +}
>[(set_attr "type" "logic_shift_imm")]
> )
>
> @@ -4504,17 +4516,6 @@ (define_split
>"operands[3] = gen_reg_rtx (mode);"
> )
>
> -(define_insn "*_rol3"
> - [(set (match_operand:GPI 0 "register_operand" "=r")
> - (LOGICAL:GPI (rotate:GPI
> - (match_operand:GPI 1 "register_operand" "r")
> - (match_operand:QI 2 "aarch64_shift_imm_" "n"))
> -(match_operand:GPI 3 "register_operand" "r")))]
> - ""
> - "\\t%0, %3, %1, ror #( - %2)"
> - [(set_attr "type" "logic_shift_imm")]
> -)
> -
> ;; zero_extend versions of above
> (define_insn "*_si3_uxtw"
>[(set (match_operand:DI 0 "register_operand" "=r")
> @@ -4524,19 +4525,11 @@ (define_insn "*_si3_uxtw"
> (match_operand:QI 2 "aarch64_shift_imm_si" "n"))
> (match_operand:SI 3 "register_operand" "r"]
>""
> - "\\t%w0, %w3, %w1, %2"
> - [(set_attr "type" "logic_shift_imm")]
> -)
> -
> -(define_insn "*_rolsi3_uxtw"
> - [(set (match_operand:DI 0 "register_operand" "=r")
> - (zero_extend:DI
> -(LOGICAL:SI (rotate:SI
> - (match_operand:SI 1 "register_operand" "r")
> - (match_operand:QI 2 "aarch64_shift_imm_si" "n"))
> -(match_operand:SI 3 "register_operand" "r"]
> - ""
> - "\\t%w0, %w3, %w1, ror #(32 - %2)"
> +{
> + if ()
> +operands[2] = GEN_INT (32 - UINTVAL (operands[2]));
> + return "\\t%w0, %w3, %w1, %2";
> +}
>[(set_attr "type" "logic_shift_imm")]
> )
>
> @@ -4565,7 +4558,11 @@ (define_insn "*one_cmpl_2"
> (not:GPI (SHIFT:GPI (match_operand:GPI 1 "register_operand" "r")
>
[PATCH] AArch64: Improve rotate patterns
Improve and generalize rotate patterns. Rotates by more than half the
bitwidth of a register are canonicalized to rotate left. Many existing
shift patterns don't handle this case correctly, so add rotate left to
the shift iterator and convert rotate left into ror during assembly
output. Add missing zero_extend patterns for shifted BIC, ORN and EON.
Passes bootstrap and regress. OK for commit?
ChangeLog:
2021-12-07 Wilco Dijkstra
* config/aarch64/aarch64.md
(and_3_compare0): Support rotate left.
(and_si3_compare0_uxtw): Likewise.
(_3): Likewise.
(_si3_uxtw): Likewise.
(one_cmpl_2): Likewise.
(_one_cmpl_3): Likewise.
(_one_cmpl_sidi_uxtw): New pattern.
(eor_one_cmpl_3_alt): Support rotate left.
(eor_one_cmpl_sidi3_alt_ze): Likewise.
(and_one_cmpl_3_compare0): Likewise.
(and_one_cmpl_si3_compare0_uxtw): Likewise.
(and_one_cmpl_3_compare0_no_reuse): Likewise.
(and_3nr_compare0): Likewise.
(*si3_insn_uxtw): Use SHIFT_no_rotate.
(rolsi3_insn_uxtw): New pattern.
* config/aarch64/iterators.md (SHIFT): Add rotate left.
(SHIFT_no_rotate): Add new iterator.
(SHIFT:shift): Print rotate left as ror.
(is_rotl): Add test for left rotate.
* gcc.target/aarch64/ror_2.c: New test.
* gcc.target/aarch64/ror_3.c: New test.
---
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index
5297b2d3f95744ac72e36814c6676cc97478d48b..f80679bcb34ea07918b1a0304b32be436568095d
100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -4419,7 +4419,11 @@ (define_insn "*and_3_compare0"
(set (match_operand:GPI 0 "register_operand" "=r")
(and:GPI (SHIFT:GPI (match_dup 1) (match_dup 2)) (match_dup 3)))]
""
- "ands\\t%0, %3, %1, %2"
+{
+ if ()
+operands[2] = GEN_INT ( - UINTVAL (operands[2]));
+ return "ands\\t%0, %3, %1, %2";
+}
[(set_attr "type" "logics_shift_imm")]
)
@@ -4436,7 +4440,11 @@ (define_insn "*and_si3_compare0_uxtw"
(zero_extend:DI (and:SI (SHIFT:SI (match_dup 1) (match_dup 2))
(match_dup 3]
""
- "ands\\t%w0, %w3, %w1, %2"
+{
+ if ()
+operands[2] = GEN_INT (32 - UINTVAL (operands[2]));
+ return "ands\\t%w0, %w3, %w1, %2";
+}
[(set_attr "type" "logics_shift_imm")]
)
@@ -4447,7 +4455,11 @@ (define_insn "*_3"
(match_operand:QI 2 "aarch64_shift_imm_" "n"))
(match_operand:GPI 3 "register_operand" "r")))]
""
- "\\t%0, %3, %1, %2"
+{
+ if ()
+operands[2] = GEN_INT ( - UINTVAL (operands[2]));
+ return "\\t%0, %3, %1, %2";
+}
[(set_attr "type" "logic_shift_imm")]
)
@@ -4504,17 +4516,6 @@ (define_split
"operands[3] = gen_reg_rtx (mode);"
)
-(define_insn "*_rol3"
- [(set (match_operand:GPI 0 "register_operand" "=r")
- (LOGICAL:GPI (rotate:GPI
- (match_operand:GPI 1 "register_operand" "r")
- (match_operand:QI 2 "aarch64_shift_imm_" "n"))
-(match_operand:GPI 3 "register_operand" "r")))]
- ""
- "\\t%0, %3, %1, ror #( - %2)"
- [(set_attr "type" "logic_shift_imm")]
-)
-
;; zero_extend versions of above
(define_insn "*_si3_uxtw"
[(set (match_operand:DI 0 "register_operand" "=r")
@@ -4524,19 +4525,11 @@ (define_insn "*_si3_uxtw"
(match_operand:QI 2 "aarch64_shift_imm_si" "n"))
(match_operand:SI 3 "register_operand" "r"]
""
- "\\t%w0, %w3, %w1, %2"
- [(set_attr "type" "logic_shift_imm")]
-)
-
-(define_insn "*_rolsi3_uxtw"
- [(set (match_operand:DI 0 "register_operand" "=r")
- (zero_extend:DI
-(LOGICAL:SI (rotate:SI
- (match_operand:SI 1 "register_operand" "r")
- (match_operand:QI 2 "aarch64_shift_imm_si" "n"))
-(match_operand:SI 3 "register_operand" "r"]
- ""
- "\\t%w0, %w3, %w1, ror #(32 - %2)"
+{
+ if ()
+operands[2] = GEN_INT (32 - UINTVAL (operands[2]));
+ return "\\t%w0, %w3, %w1, %2";
+}
[(set_attr "type" "logic_shift_imm")]
)
@@ -4565,7 +4558,11 @@ (define_insn "*one_cmpl_2"
(not:GPI (SHIFT:GPI (match_operand:GPI 1 "register_operand" "r")
(match_operand:QI 2 "aarch64_shift_imm_"
"n"]
""
- "mvn\\t%0, %1, %2"
+{
+ if ()
+operands[2] = GEN_INT ( - UINTVAL (operands[2]));
+ return "mvn\\t%0, %1, %2";
+}
[(set_attr "type" "logic_shift_imm")]
)
@@ -4672,7 +4669,28 @@ (define_insn
"_one_cmpl_3"
(match_operand:QI 2 "aarch64_shift_imm_" "n")))
(match_operand:GPI 3 "register_operand" "r")))]
""
- "\\t%0, %3, %1, %2"
+{
+ if ()
+operands[2] = GEN_INT ( - UINTVAL (operands[2]));
+ return "\\t%0, %3, %1, %2";
+}
+ [(set_attr "type" "logic_shift_imm")]
+)
+
+;; Zero-extend version of the above.
+(define_insn "_one_cmpl_sidi_uxtw"
+ [(set
