[PATCH][AArch64][2/3] PR target/84164: Add ZERO_EXTRACT + LSHIFTRT pattern for BFXIL instruction

[PATCH][AArch64][2/3] PR target/84164: Add ZERO_EXTRACT + LSHIFTRT pattern for BFXIL instruction

[Kyrill Tkachov]

[-- Attachment #1: Type: text/plain, Size: 1304 bytes --]

Hi all,

This is a followup to the other PR target/84164 patch [1] that fixes the testsuite regression
gcc.target/aarch64/bfxil_1.c.
The regression is that with the new subreg+masking simplification we no longer match the
pattern for BFXIL that has the form:
(set (zero_extract:DI (reg/v:DI 76 [ a ])
         (const_int 8 [0x8])
         (const_int 0 [0]))
     (zero_extract:DI (reg/v:DI 76 [ a ])
         (const_int 8 [0x8])
         (const_int 16 [0x10])))

This is now instead represented as:
(set (zero_extract:DI (reg/v:DI 93 [ a ])
         (const_int 8 [0x8])
         (const_int 0 [0]))
     (lshiftrt:DI (reg/v:DI 93 [ a ])
         (const_int 16 [0x10])))

As far as I can see the two are equivalent semantically and the LSHIFTRT form is a bit
simpler, so I think the simplified form is valid, but we have no pattern to match it.
This patch adds that pattern to catch this form as well.
This fixes the aforementioned regression and bootstrap and testing on aarch64-none-linux-gnu
shows no problem.

Is this ok for trunk if the first patch goes in?
Thanks,
Kyrill

[1] https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00443.html

2018-02-08  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR target/84164
     * config/aarch64/aarch64.md (*extr_insv_lower_reg_lshiftrt<mode>):
     New pattern.

[-- Attachment #2: aarch64-bfxil-2.patch --]
[-- Type: text/x-patch, Size: 978 bytes --]

diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index 62a4f8262a316087894aeb555c609fbe75885203..2c6343a363c161ed503ca1ddd752e21b5c941eb8 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -4803,6 +4803,19 @@ (define_insn "*extr_insv_lower_reg<mode>"
   [(set_attr "type" "bfm")]
 )
 
+(define_insn "*extr_insv_lower_reg_lshiftrt<mode>"
+  [(set (zero_extract:GPI (match_operand:GPI 0 "register_operand" "+r")
+			  (match_operand 1 "const_int_operand" "n")
+			  (const_int 0))
+	(lshiftrt:GPI (match_operand:GPI 2 "register_operand" "r")
+			(match_operand 3 "const_int_operand" "n")))]
+  "!(UINTVAL (operands[1]) == 0
+     || (UINTVAL (operands[3]) + UINTVAL (operands[1])
+	 > GET_MODE_BITSIZE (<MODE>mode)))"
+  "bfxil\\t%<w>0, %<w>2, %3, %1"
+  [(set_attr "type" "bfm")]
+)
+
 (define_insn "*<optab><ALLX:mode>_shft_<GPI:mode>"
   [(set (match_operand:GPI 0 "register_operand" "=r")
 	(ashift:GPI (ANY_EXTEND:GPI

Re: [PATCH][AArch64][2/3] PR target/84164: Add ZERO_EXTRACT + LSHIFTRT pattern for BFXIL instruction

[Kyrill Tkachov]

Hi all,

I'm pinging this patch (https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00444.html)
now that Jeff has approved the prerequisite simplify-rtx.c change (https://gcc.gnu.org/ml/gcc-patches/2018-03/msg00143.html) (thanks!)

Thanks,
Kyrill

On 08/02/18 17:10, Kyrill Tkachov wrote:
> Hi all,
>
> This is a followup to the other PR target/84164 patch [1] that fixes the testsuite regression
> gcc.target/aarch64/bfxil_1.c.
> The regression is that with the new subreg+masking simplification we no longer match the
> pattern for BFXIL that has the form:
> (set (zero_extract:DI (reg/v:DI 76 [ a ])
>         (const_int 8 [0x8])
>         (const_int 0 [0]))
>     (zero_extract:DI (reg/v:DI 76 [ a ])
>         (const_int 8 [0x8])
>         (const_int 16 [0x10])))
>
> This is now instead represented as:
> (set (zero_extract:DI (reg/v:DI 93 [ a ])
>         (const_int 8 [0x8])
>         (const_int 0 [0]))
>     (lshiftrt:DI (reg/v:DI 93 [ a ])
>         (const_int 16 [0x10])))
>
> As far as I can see the two are equivalent semantically and the LSHIFTRT form is a bit
> simpler, so I think the simplified form is valid, but we have no pattern to match it.
> This patch adds that pattern to catch this form as well.
> This fixes the aforementioned regression and bootstrap and testing on aarch64-none-linux-gnu
> shows no problem.
>
> Is this ok for trunk if the first patch goes in?
> Thanks,
> Kyrill
>
> [1] https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00443.html
>
> 2018-02-08  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     PR target/84164
>     * config/aarch64/aarch64.md (*extr_insv_lower_reg_lshiftrt<mode>):
>     New pattern.

Re: [PATCH][AArch64][2/3] PR target/84164: Add ZERO_EXTRACT + LSHIFTRT pattern for BFXIL instruction

[James Greenhalgh]

On Thu, Feb 08, 2018 at 05:10:52PM +0000, Kyrill Tkachov wrote:
> Hi all,
> 
> This is a followup to the other PR target/84164 patch [1] that fixes the testsuite regression
> gcc.target/aarch64/bfxil_1.c.
> The regression is that with the new subreg+masking simplification we no longer match the
> pattern for BFXIL that has the form:
> (set (zero_extract:DI (reg/v:DI 76 [ a ])
>          (const_int 8 [0x8])
>          (const_int 0 [0]))
>      (zero_extract:DI (reg/v:DI 76 [ a ])
>          (const_int 8 [0x8])
>          (const_int 16 [0x10])))
> 
> This is now instead represented as:
> (set (zero_extract:DI (reg/v:DI 93 [ a ])
>          (const_int 8 [0x8])
>          (const_int 0 [0]))
>      (lshiftrt:DI (reg/v:DI 93 [ a ])
>          (const_int 16 [0x10])))
> 
> As far as I can see the two are equivalent semantically and the LSHIFTRT form is a bit
> simpler, so I think the simplified form is valid, but we have no pattern to match it.
> This patch adds that pattern to catch this form as well.
> This fixes the aforementioned regression and bootstrap and testing on aarch64-none-linux-gnu
> shows no problem.
> 
> Is this ok for trunk if the first patch goes in?

OK.

Thanks,
James

> [1] https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00443.html
> 
> 2018-02-08  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>      PR target/84164
>      * config/aarch64/aarch64.md (*extr_insv_lower_reg_lshiftrt<mode>):
>      New pattern.

> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 62a4f8262a316087894aeb555c609fbe75885203..2c6343a363c161ed503ca1ddd752e21b5c941eb8 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -4803,6 +4803,19 @@ (define_insn "*extr_insv_lower_reg<mode>"
>    [(set_attr "type" "bfm")]
>  )
>  
> +(define_insn "*extr_insv_lower_reg_lshiftrt<mode>"
> +  [(set (zero_extract:GPI (match_operand:GPI 0 "register_operand" "+r")
> +			  (match_operand 1 "const_int_operand" "n")
> +			  (const_int 0))
> +	(lshiftrt:GPI (match_operand:GPI 2 "register_operand" "r")
> +			(match_operand 3 "const_int_operand" "n")))]
> +  "!(UINTVAL (operands[1]) == 0
> +     || (UINTVAL (operands[3]) + UINTVAL (operands[1])
> +	 > GET_MODE_BITSIZE (<MODE>mode)))"
> +  "bfxil\\t%<w>0, %<w>2, %3, %1"
> +  [(set_attr "type" "bfm")]
> +)
> +
>  (define_insn "*<optab><ALLX:mode>_shft_<GPI:mode>"
>    [(set (match_operand:GPI 0 "register_operand" "=r")
>  	(ashift:GPI (ANY_EXTEND:GPI