* [PATCH] aarch64: Fix up 2 other combine opt regressions vs. GCC8 [PR100075]
@ 2021-04-15 15:43 Jakub Jelinek
2021-04-15 18:11 ` Richard Sandiford
0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2021-04-15 15:43 UTC (permalink / raw)
To: Richard Earnshaw, Richard Sandiford, Marcus Shawcroft, Kyrylo Tkachov
Cc: gcc-patches
Hi!
The testcase used to be compiled at -O2 by GCC8 and earlier to:
f1:
neg w1, w0, asr 16
and w1, w1, 65535
orr w0, w1, w0, lsl 16
ret
f2:
neg w1, w0
extr w0, w1, w0, 16
ret
but since GCC9 (r9-3594 for f1 and r9-6926 for f2) we compile it into:
f1:
mov w1, w0
sbfx x0, x1, 16, 16
neg w0, w0
bfi w0, w1, 16, 16
ret
f2:
neg w1, w0
sbfx x0, x0, 16, 16
bfi w0, w1, 16, 16
ret
instead, i.e. one insn longer each. With this patch we get:
f1:
mov w1, w0
neg w0, w1, asr 16
bfi w0, w1, 16, 16
ret
f2:
neg w1, w0
extr w0, w1, w0, 16
ret
i.e. identical f2 and same number of insns as in GCC8 in f1.
The combiner unfortunately doesn't try splitters when doing 2 -> 1
combination, so it can't be implemented as combine splitters, but
it could be implemented as define_insn_and_split if desirable.
Bootstrapped/regtested on aarch64-linux, ok for trunk?
2021-04-15 Jakub Jelinek <jakub@redhat.com>
PR target/100075
* config/aarch64/aarch64.md (*neg_asr_si2_extr, *extrsi5_insn_di): New
define_insn patterns.
* gcc.target/aarch64/pr100075.c: New test.
--- gcc/config/aarch64/aarch64.md.jj 2021-04-15 10:45:02.798853095 +0200
+++ gcc/config/aarch64/aarch64.md 2021-04-15 13:28:04.734754364 +0200
@@ -3572,6 +3572,18 @@ (define_insn "*neg_<shift>_si2_uxtw"
[(set_attr "autodetect_type" "alu_shift_<shift>_op2")]
)
+(define_insn "*neg_asr_si2_extr"
+ [(set (match_operand:SI 0 "register_operand" "r")
+ (neg:SI (match_operator 4 "subreg_lowpart_operator"
+ [(sign_extract:DI
+ (match_operand:DI 1 "register_operand" "r")
+ (match_operand 3 "aarch64_simd_shift_imm_offset_si" "n")
+ (match_operand 2 "aarch64_simd_shift_imm_offset_si" "n"))])))]
+ "INTVAL (operands[2]) + INTVAL (operands[3]) == 32"
+ "neg\\t%w0, %w1, asr %2"
+ [(set_attr "autodetect_type" "alu_shift_asr_op2")]
+)
+
(define_insn "mul<mode>3"
[(set (match_operand:GPI 0 "register_operand" "=r")
(mult:GPI (match_operand:GPI 1 "register_operand" "r")
@@ -5382,6 +5394,22 @@ (define_insn "*extrsi5_insn_uxtw_alt"
"extr\\t%w0, %w1, %w2, %4"
[(set_attr "type" "rotate_imm")]
)
+
+(define_insn "*extrsi5_insn_di"
+ [(set (match_operand:SI 0 "register_operand" "=r")
+ (ior:SI (ashift:SI (match_operand:SI 1 "register_operand" "r")
+ (match_operand 3 "const_int_operand" "n"))
+ (match_operator:SI 6 "subreg_lowpart_operator"
+ [(zero_extract:DI
+ (match_operand:DI 2 "register_operand" "r")
+ (match_operand 5 "const_int_operand" "n")
+ (match_operand 4 "const_int_operand" "n"))])))]
+ "UINTVAL (operands[3]) < 32
+ && UINTVAL (operands[3]) + UINTVAL (operands[4]) == 32
+ && UINTVAL (operands[4]) + UINTVAL (operands[5]) - 32 <= 64"
+ "extr\\t%w0, %w1, %w2, %4"
+ [(set_attr "type" "rotate_imm")]
+)
(define_insn "*ror<mode>3_insn"
[(set (match_operand:GPI 0 "register_operand" "=r")
--- gcc/testsuite/gcc.target/aarch64/pr100075.c.jj 2021-04-15 13:23:31.188852983 +0200
+++ gcc/testsuite/gcc.target/aarch64/pr100075.c 2021-04-15 13:23:10.612086048 +0200
@@ -0,0 +1,20 @@
+/* PR target/100075 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-final { scan-assembler-not {\tsbfx\tx[0-9]+, x[0-9]+, 16, 16} } } */
+/* { dg-final { scan-assembler {\tneg\tw[0-9]+, w[0-9]+, asr 16} } } */
+/* { dg-final { scan-assembler {\textr\tw[0-9]+, w[0-9]+, w[0-9]+, 16} } } */
+
+struct S { short x, y; };
+
+struct S
+f1 (struct S p)
+{
+ return (struct S) { -p.y, p.x };
+}
+
+struct S
+f2 (struct S p)
+{
+ return (struct S) { p.y, -p.x };
+}
Jakub
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] aarch64: Fix up 2 other combine opt regressions vs. GCC8 [PR100075]
2021-04-15 15:43 [PATCH] aarch64: Fix up 2 other combine opt regressions vs. GCC8 [PR100075] Jakub Jelinek
@ 2021-04-15 18:11 ` Richard Sandiford
2021-04-15 18:51 ` Jakub Jelinek
0 siblings, 1 reply; 7+ messages in thread
From: Richard Sandiford @ 2021-04-15 18:11 UTC (permalink / raw)
To: Jakub Jelinek
Cc: Richard Earnshaw, Marcus Shawcroft, Kyrylo Tkachov, gcc-patches
Jakub Jelinek <jakub@redhat.com> writes:
> --- gcc/config/aarch64/aarch64.md.jj 2021-04-15 10:45:02.798853095 +0200
> +++ gcc/config/aarch64/aarch64.md 2021-04-15 13:28:04.734754364 +0200
> @@ -3572,6 +3572,18 @@ (define_insn "*neg_<shift>_si2_uxtw"
> [(set_attr "autodetect_type" "alu_shift_<shift>_op2")]
> )
>
> +(define_insn "*neg_asr_si2_extr"
> + [(set (match_operand:SI 0 "register_operand" "r")
> + (neg:SI (match_operator 4 "subreg_lowpart_operator"
Very minor, but it might be better to have the :SI on the match_operator
too, like in the pattern below.
> + [(sign_extract:DI
> + (match_operand:DI 1 "register_operand" "r")
> + (match_operand 3 "aarch64_simd_shift_imm_offset_si" "n")
> + (match_operand 2 "aarch64_simd_shift_imm_offset_si" "n"))])))]
> + "INTVAL (operands[2]) + INTVAL (operands[3]) == 32"
> + "neg\\t%w0, %w1, asr %2"
> + [(set_attr "autodetect_type" "alu_shift_asr_op2")]
> +)
> +
> (define_insn "mul<mode>3"
> [(set (match_operand:GPI 0 "register_operand" "=r")
> (mult:GPI (match_operand:GPI 1 "register_operand" "r")
> @@ -5382,6 +5394,22 @@ (define_insn "*extrsi5_insn_uxtw_alt"
> "extr\\t%w0, %w1, %w2, %4"
> [(set_attr "type" "rotate_imm")]
> )
> +
> +(define_insn "*extrsi5_insn_di"
> + [(set (match_operand:SI 0 "register_operand" "=r")
> + (ior:SI (ashift:SI (match_operand:SI 1 "register_operand" "r")
> + (match_operand 3 "const_int_operand" "n"))
> + (match_operator:SI 6 "subreg_lowpart_operator"
> + [(zero_extract:DI
> + (match_operand:DI 2 "register_operand" "r")
> + (match_operand 5 "const_int_operand" "n")
> + (match_operand 4 "const_int_operand" "n"))])))]
> + "UINTVAL (operands[3]) < 32
> + && UINTVAL (operands[3]) + UINTVAL (operands[4]) == 32
> + && UINTVAL (operands[4]) + UINTVAL (operands[5]) - 32 <= 64"
Could you explain this condition? With operand 5 being the size
and operand 4 being the position, I was expecting something like:
"UINTVAL (operands[3]) < 32
&& UINTVAL (operands[3]) == UINTVAL (operands[5])
&& UINTVAL (operands[4]) + UINTVAL (operands[5]) == 32"
i.e. the %w1 shift must equal the size of the %w2 extraction
and the %w2 extraction must align with the top of the register.
Or, writing it in more the style of the original condition,
the final line would be:
&& UINTVAL (operands[3]) == UINTVAL (operands[5])"
instead of:
&& UINTVAL (operands[4]) + UINTVAL (operands[5]) - 32 <= 64"
Not tested though, and it's late, so I could have got that completely
wrong :-)
Thanks,
Richard
> + "extr\\t%w0, %w1, %w2, %4"
> + [(set_attr "type" "rotate_imm")]
> +)
>
> (define_insn "*ror<mode>3_insn"
> [(set (match_operand:GPI 0 "register_operand" "=r")
> --- gcc/testsuite/gcc.target/aarch64/pr100075.c.jj 2021-04-15 13:23:31.188852983 +0200
> +++ gcc/testsuite/gcc.target/aarch64/pr100075.c 2021-04-15 13:23:10.612086048 +0200
> @@ -0,0 +1,20 @@
> +/* PR target/100075 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +/* { dg-final { scan-assembler-not {\tsbfx\tx[0-9]+, x[0-9]+, 16, 16} } } */
> +/* { dg-final { scan-assembler {\tneg\tw[0-9]+, w[0-9]+, asr 16} } } */
> +/* { dg-final { scan-assembler {\textr\tw[0-9]+, w[0-9]+, w[0-9]+, 16} } } */
> +
> +struct S { short x, y; };
> +
> +struct S
> +f1 (struct S p)
> +{
> + return (struct S) { -p.y, p.x };
> +}
> +
> +struct S
> +f2 (struct S p)
> +{
> + return (struct S) { p.y, -p.x };
> +}
>
> Jakub
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] aarch64: Fix up 2 other combine opt regressions vs. GCC8 [PR100075]
2021-04-15 18:11 ` Richard Sandiford
@ 2021-04-15 18:51 ` Jakub Jelinek
2021-04-16 7:38 ` [PATCH] aarch64, v2: " Jakub Jelinek
0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2021-04-15 18:51 UTC (permalink / raw)
To: Richard Earnshaw, Marcus Shawcroft, Kyrylo Tkachov, gcc-patches,
richard.sandiford
On Thu, Apr 15, 2021 at 07:11:11PM +0100, Richard Sandiford wrote:
> Jakub Jelinek <jakub@redhat.com> writes:
> > --- gcc/config/aarch64/aarch64.md.jj 2021-04-15 10:45:02.798853095 +0200
> > +++ gcc/config/aarch64/aarch64.md 2021-04-15 13:28:04.734754364 +0200
> > @@ -3572,6 +3572,18 @@ (define_insn "*neg_<shift>_si2_uxtw"
> > [(set_attr "autodetect_type" "alu_shift_<shift>_op2")]
> > )
> >
> > +(define_insn "*neg_asr_si2_extr"
> > + [(set (match_operand:SI 0 "register_operand" "r")
> > + (neg:SI (match_operator 4 "subreg_lowpart_operator"
>
> Very minor, but it might be better to have the :SI on the match_operator
> too, like in the pattern below.
Fixed, thanks for catching that (and the "r" -> "=r"; I've
actually tested a patch that didn't have any constraints on the first
define_insn because I started with a define_split that didn't work,
and it happened to work during testing, only noticed I've missed them
afterwards.
> > @@ -5382,6 +5394,22 @@ (define_insn "*extrsi5_insn_uxtw_alt"
> > "extr\\t%w0, %w1, %w2, %4"
> > [(set_attr "type" "rotate_imm")]
> > )
> > +
> > +(define_insn "*extrsi5_insn_di"
> > + [(set (match_operand:SI 0 "register_operand" "=r")
> > + (ior:SI (ashift:SI (match_operand:SI 1 "register_operand" "r")
> > + (match_operand 3 "const_int_operand" "n"))
> > + (match_operator:SI 6 "subreg_lowpart_operator"
> > + [(zero_extract:DI
> > + (match_operand:DI 2 "register_operand" "r")
> > + (match_operand 5 "const_int_operand" "n")
> > + (match_operand 4 "const_int_operand" "n"))])))]
> > + "UINTVAL (operands[3]) < 32
> > + && UINTVAL (operands[3]) + UINTVAL (operands[4]) == 32
> > + && UINTVAL (operands[4]) + UINTVAL (operands[5]) - 32 <= 64"
>
> Could you explain this condition? With operand 5 being the size
> and operand 4 being the position, I was expecting something like:
The first two conditions are like those on *extr<mode>5_insn
except that GET_MODE_BITSIZE (<MODE>mode) is hardcoded as 32.
Like in *extr<mode>5_insn, we have two shift counts, because
aarch64 is !BITS_BIG_ENDIAN the last operand of zero_extract
is a shift count too.
The
UINTVAL (operands[4]) + UINTVAL (operands[5]) - 32 <= 64
(should have been <= 32 actually) meant to test
IN_RANGE (UINTVAL (operands[4]) + UINTVAL (operands[5]), 32, 64)
because I mistakenly thought
or maybe just
UINTVAL (operands[4]) + UINTVAL (operands[5]) >= 32
was meant to test that the zero_extract extracts at least as many bits
that it doesn't mask any bits above that. But the subreg:SI
actually applies to the zero_extract result and therefore
operands[2] >> operands[4] rather than before that, so you're right
it needs to be == 32 rather than >= 32.
So, either it can be
"UINTVAL (operands[3]) < 32
&& UINTVAL (operands[3]) + UINTVAL (operands[4]) == 32
&& UINTVAL (operands[4]) + UINTVAL (operands[5]) == 32"
or what you wrote:
> "UINTVAL (operands[3]) < 32
> && UINTVAL (operands[3]) == UINTVAL (operands[5])
> && UINTVAL (operands[4]) + UINTVAL (operands[5]) == 32"
or it could be:
"UINTVAL (operands[3]) < 32
&& UINTVAL (operands[3]) + UINTVAL (operands[4]) == 32
&& INTVAL (operands[3]) == INTVAL (operands[5])"
Jakub
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] aarch64, v2: Fix up 2 other combine opt regressions vs. GCC8 [PR100075]
2021-04-15 18:51 ` Jakub Jelinek
@ 2021-04-16 7:38 ` Jakub Jelinek
2021-04-16 11:29 ` Richard Sandiford
0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2021-04-16 7:38 UTC (permalink / raw)
To: Richard Earnshaw, Marcus Shawcroft, Kyrylo Tkachov, richard.sandiford
Cc: gcc-patches
On Thu, Apr 15, 2021 at 08:51:03PM +0200, Jakub Jelinek via Gcc-patches wrote:
> Fixed, thanks for catching that (and the "r" -> "=r"; I've
> actually tested a patch that didn't have any constraints on the first
> define_insn because I started with a define_split that didn't work,
> and it happened to work during testing, only noticed I've missed them
> afterwards.
> or it could be:
> "UINTVAL (operands[3]) < 32
> && UINTVAL (operands[3]) + UINTVAL (operands[4]) == 32
> && INTVAL (operands[3]) == INTVAL (operands[5])"
Here is an updated patch that I've also successfully bootstrapped/regtested
on aarch64-linux.
2021-04-16 Jakub Jelinek <jakub@redhat.com>
PR target/100075
* config/aarch64/aarch64.md (*neg_asr_si2_extr, *extrsi5_insn_di): New
define_insn patterns.
* gcc.target/aarch64/pr100075.c: New test.
--- gcc/config/aarch64/aarch64.md.jj 2021-04-15 10:45:02.798853095 +0200
+++ gcc/config/aarch64/aarch64.md 2021-04-15 13:28:04.734754364 +0200
@@ -3572,6 +3572,18 @@ (define_insn "*neg_<shift>_si2_uxtw"
[(set_attr "autodetect_type" "alu_shift_<shift>_op2")]
)
+(define_insn "*neg_asr_si2_extr"
+ [(set (match_operand:SI 0 "register_operand" "=r")
+ (neg:SI (match_operator:SI 4 "subreg_lowpart_operator"
+ [(sign_extract:DI
+ (match_operand:DI 1 "register_operand" "r")
+ (match_operand 3 "aarch64_simd_shift_imm_offset_si" "n")
+ (match_operand 2 "aarch64_simd_shift_imm_offset_si" "n"))])))]
+ "INTVAL (operands[2]) + INTVAL (operands[3]) == 32"
+ "neg\\t%w0, %w1, asr %2"
+ [(set_attr "autodetect_type" "alu_shift_asr_op2")]
+)
+
(define_insn "mul<mode>3"
[(set (match_operand:GPI 0 "register_operand" "=r")
(mult:GPI (match_operand:GPI 1 "register_operand" "r")
@@ -5382,6 +5394,22 @@ (define_insn "*extrsi5_insn_uxtw_alt"
"extr\\t%w0, %w1, %w2, %4"
[(set_attr "type" "rotate_imm")]
)
+
+(define_insn "*extrsi5_insn_di"
+ [(set (match_operand:SI 0 "register_operand" "=r")
+ (ior:SI (ashift:SI (match_operand:SI 1 "register_operand" "r")
+ (match_operand 3 "const_int_operand" "n"))
+ (match_operator:SI 6 "subreg_lowpart_operator"
+ [(zero_extract:DI
+ (match_operand:DI 2 "register_operand" "r")
+ (match_operand 5 "const_int_operand" "n")
+ (match_operand 4 "const_int_operand" "n"))])))]
+ "UINTVAL (operands[3]) < 32
+ && UINTVAL (operands[3]) + UINTVAL (operands[4]) == 32
+ && INTVAL (operands[3]) == INTVAL (operands[5])"
+ "extr\\t%w0, %w1, %w2, %4"
+ [(set_attr "type" "rotate_imm")]
+)
(define_insn "*ror<mode>3_insn"
[(set (match_operand:GPI 0 "register_operand" "=r")
--- gcc/testsuite/gcc.target/aarch64/pr100075.c.jj 2021-04-15 13:23:31.188852983 +0200
+++ gcc/testsuite/gcc.target/aarch64/pr100075.c 2021-04-15 13:23:10.612086048 +0200
@@ -0,0 +1,20 @@
+/* PR target/100075 */
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+/* { dg-final { scan-assembler-not {\tsbfx\tx[0-9]+, x[0-9]+, 16, 16} } } */
+/* { dg-final { scan-assembler {\tneg\tw[0-9]+, w[0-9]+, asr 16} } } */
+/* { dg-final { scan-assembler {\textr\tw[0-9]+, w[0-9]+, w[0-9]+, 16} } } */
+
+struct S { short x, y; };
+
+struct S
+f1 (struct S p)
+{
+ return (struct S) { -p.y, p.x };
+}
+
+struct S
+f2 (struct S p)
+{
+ return (struct S) { p.y, -p.x };
+}
Jakub
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] aarch64, v2: Fix up 2 other combine opt regressions vs. GCC8 [PR100075]
2021-04-16 7:38 ` [PATCH] aarch64, v2: " Jakub Jelinek
@ 2021-04-16 11:29 ` Richard Sandiford
2021-04-19 11:16 ` Christophe Lyon
0 siblings, 1 reply; 7+ messages in thread
From: Richard Sandiford @ 2021-04-16 11:29 UTC (permalink / raw)
To: Jakub Jelinek
Cc: Richard Earnshaw, Marcus Shawcroft, Kyrylo Tkachov, gcc-patches
Jakub Jelinek <jakub@redhat.com> writes:
> On Thu, Apr 15, 2021 at 08:51:03PM +0200, Jakub Jelinek via Gcc-patches wrote:
>> Fixed, thanks for catching that (and the "r" -> "=r"; I've
>> actually tested a patch that didn't have any constraints on the first
>> define_insn because I started with a define_split that didn't work,
>> and it happened to work during testing, only noticed I've missed them
>> afterwards.
>
>> or it could be:
>> "UINTVAL (operands[3]) < 32
>> && UINTVAL (operands[3]) + UINTVAL (operands[4]) == 32
>> && INTVAL (operands[3]) == INTVAL (operands[5])"
>
> Here is an updated patch that I've also successfully bootstrapped/regtested
> on aarch64-linux.
>
> 2021-04-16 Jakub Jelinek <jakub@redhat.com>
>
> PR target/100075
> * config/aarch64/aarch64.md (*neg_asr_si2_extr, *extrsi5_insn_di): New
> define_insn patterns.
>
> * gcc.target/aarch64/pr100075.c: New test.
OK, thanks.
Richard
> --- gcc/config/aarch64/aarch64.md.jj 2021-04-15 10:45:02.798853095 +0200
> +++ gcc/config/aarch64/aarch64.md 2021-04-15 13:28:04.734754364 +0200
> @@ -3572,6 +3572,18 @@ (define_insn "*neg_<shift>_si2_uxtw"
> [(set_attr "autodetect_type" "alu_shift_<shift>_op2")]
> )
>
> +(define_insn "*neg_asr_si2_extr"
> + [(set (match_operand:SI 0 "register_operand" "=r")
> + (neg:SI (match_operator:SI 4 "subreg_lowpart_operator"
> + [(sign_extract:DI
> + (match_operand:DI 1 "register_operand" "r")
> + (match_operand 3 "aarch64_simd_shift_imm_offset_si" "n")
> + (match_operand 2 "aarch64_simd_shift_imm_offset_si" "n"))])))]
> + "INTVAL (operands[2]) + INTVAL (operands[3]) == 32"
> + "neg\\t%w0, %w1, asr %2"
> + [(set_attr "autodetect_type" "alu_shift_asr_op2")]
> +)
> +
> (define_insn "mul<mode>3"
> [(set (match_operand:GPI 0 "register_operand" "=r")
> (mult:GPI (match_operand:GPI 1 "register_operand" "r")
> @@ -5382,6 +5394,22 @@ (define_insn "*extrsi5_insn_uxtw_alt"
> "extr\\t%w0, %w1, %w2, %4"
> [(set_attr "type" "rotate_imm")]
> )
> +
> +(define_insn "*extrsi5_insn_di"
> + [(set (match_operand:SI 0 "register_operand" "=r")
> + (ior:SI (ashift:SI (match_operand:SI 1 "register_operand" "r")
> + (match_operand 3 "const_int_operand" "n"))
> + (match_operator:SI 6 "subreg_lowpart_operator"
> + [(zero_extract:DI
> + (match_operand:DI 2 "register_operand" "r")
> + (match_operand 5 "const_int_operand" "n")
> + (match_operand 4 "const_int_operand" "n"))])))]
> + "UINTVAL (operands[3]) < 32
> + && UINTVAL (operands[3]) + UINTVAL (operands[4]) == 32
> + && INTVAL (operands[3]) == INTVAL (operands[5])"
> + "extr\\t%w0, %w1, %w2, %4"
> + [(set_attr "type" "rotate_imm")]
> +)
>
> (define_insn "*ror<mode>3_insn"
> [(set (match_operand:GPI 0 "register_operand" "=r")
> --- gcc/testsuite/gcc.target/aarch64/pr100075.c.jj 2021-04-15 13:23:31.188852983 +0200
> +++ gcc/testsuite/gcc.target/aarch64/pr100075.c 2021-04-15 13:23:10.612086048 +0200
> @@ -0,0 +1,20 @@
> +/* PR target/100075 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +/* { dg-final { scan-assembler-not {\tsbfx\tx[0-9]+, x[0-9]+, 16, 16} } } */
> +/* { dg-final { scan-assembler {\tneg\tw[0-9]+, w[0-9]+, asr 16} } } */
> +/* { dg-final { scan-assembler {\textr\tw[0-9]+, w[0-9]+, w[0-9]+, 16} } } */
> +
> +struct S { short x, y; };
> +
> +struct S
> +f1 (struct S p)
> +{
> + return (struct S) { -p.y, p.x };
> +}
> +
> +struct S
> +f2 (struct S p)
> +{
> + return (struct S) { p.y, -p.x };
> +}
>
>
> Jakub
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] aarch64, v2: Fix up 2 other combine opt regressions vs. GCC8 [PR100075]
2021-04-16 11:29 ` Richard Sandiford
@ 2021-04-19 11:16 ` Christophe Lyon
2021-04-19 11:44 ` Jakub Jelinek
0 siblings, 1 reply; 7+ messages in thread
From: Christophe Lyon @ 2021-04-19 11:16 UTC (permalink / raw)
To: Richard Sandiford, Jakub Jelinek, Richard Earnshaw,
Marcus Shawcroft, Kyrylo Tkachov, gcc Patches
On Fri, 16 Apr 2021 at 13:29, Richard Sandiford via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Jakub Jelinek <jakub@redhat.com> writes:
> > On Thu, Apr 15, 2021 at 08:51:03PM +0200, Jakub Jelinek via Gcc-patches wrote:
> >> Fixed, thanks for catching that (and the "r" -> "=r"; I've
> >> actually tested a patch that didn't have any constraints on the first
> >> define_insn because I started with a define_split that didn't work,
> >> and it happened to work during testing, only noticed I've missed them
> >> afterwards.
> >
> >> or it could be:
> >> "UINTVAL (operands[3]) < 32
> >> && UINTVAL (operands[3]) + UINTVAL (operands[4]) == 32
> >> && INTVAL (operands[3]) == INTVAL (operands[5])"
> >
> > Here is an updated patch that I've also successfully bootstrapped/regtested
> > on aarch64-linux.
> >
> > 2021-04-16 Jakub Jelinek <jakub@redhat.com>
> >
> > PR target/100075
> > * config/aarch64/aarch64.md (*neg_asr_si2_extr, *extrsi5_insn_di): New
> > define_insn patterns.
> >
> > * gcc.target/aarch64/pr100075.c: New test.
>
> OK, thanks.
>
Hi,
The new test fails on aarch64_be, OK to add:
+/* { dg-require-effective-target aarch64_little_endian } */
?
The testcase looks really endianness dependent.
Thanks,
Christophe
> Richard
>
> > --- gcc/config/aarch64/aarch64.md.jj 2021-04-15 10:45:02.798853095 +0200
> > +++ gcc/config/aarch64/aarch64.md 2021-04-15 13:28:04.734754364 +0200
> > @@ -3572,6 +3572,18 @@ (define_insn "*neg_<shift>_si2_uxtw"
> > [(set_attr "autodetect_type" "alu_shift_<shift>_op2")]
> > )
> >
> > +(define_insn "*neg_asr_si2_extr"
> > + [(set (match_operand:SI 0 "register_operand" "=r")
> > + (neg:SI (match_operator:SI 4 "subreg_lowpart_operator"
> > + [(sign_extract:DI
> > + (match_operand:DI 1 "register_operand" "r")
> > + (match_operand 3 "aarch64_simd_shift_imm_offset_si" "n")
> > + (match_operand 2 "aarch64_simd_shift_imm_offset_si" "n"))])))]
> > + "INTVAL (operands[2]) + INTVAL (operands[3]) == 32"
> > + "neg\\t%w0, %w1, asr %2"
> > + [(set_attr "autodetect_type" "alu_shift_asr_op2")]
> > +)
> > +
> > (define_insn "mul<mode>3"
> > [(set (match_operand:GPI 0 "register_operand" "=r")
> > (mult:GPI (match_operand:GPI 1 "register_operand" "r")
> > @@ -5382,6 +5394,22 @@ (define_insn "*extrsi5_insn_uxtw_alt"
> > "extr\\t%w0, %w1, %w2, %4"
> > [(set_attr "type" "rotate_imm")]
> > )
> > +
> > +(define_insn "*extrsi5_insn_di"
> > + [(set (match_operand:SI 0 "register_operand" "=r")
> > + (ior:SI (ashift:SI (match_operand:SI 1 "register_operand" "r")
> > + (match_operand 3 "const_int_operand" "n"))
> > + (match_operator:SI 6 "subreg_lowpart_operator"
> > + [(zero_extract:DI
> > + (match_operand:DI 2 "register_operand" "r")
> > + (match_operand 5 "const_int_operand" "n")
> > + (match_operand 4 "const_int_operand" "n"))])))]
> > + "UINTVAL (operands[3]) < 32
> > + && UINTVAL (operands[3]) + UINTVAL (operands[4]) == 32
> > + && INTVAL (operands[3]) == INTVAL (operands[5])"
> > + "extr\\t%w0, %w1, %w2, %4"
> > + [(set_attr "type" "rotate_imm")]
> > +)
> >
> > (define_insn "*ror<mode>3_insn"
> > [(set (match_operand:GPI 0 "register_operand" "=r")
> > --- gcc/testsuite/gcc.target/aarch64/pr100075.c.jj 2021-04-15 13:23:31.188852983 +0200
> > +++ gcc/testsuite/gcc.target/aarch64/pr100075.c 2021-04-15 13:23:10.612086048 +0200
> > @@ -0,0 +1,20 @@
> > +/* PR target/100075 */
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2" } */
> > +/* { dg-final { scan-assembler-not {\tsbfx\tx[0-9]+, x[0-9]+, 16, 16} } } */
> > +/* { dg-final { scan-assembler {\tneg\tw[0-9]+, w[0-9]+, asr 16} } } */
> > +/* { dg-final { scan-assembler {\textr\tw[0-9]+, w[0-9]+, w[0-9]+, 16} } } */
> > +
> > +struct S { short x, y; };
> > +
> > +struct S
> > +f1 (struct S p)
> > +{
> > + return (struct S) { -p.y, p.x };
> > +}
> > +
> > +struct S
> > +f2 (struct S p)
> > +{
> > + return (struct S) { p.y, -p.x };
> > +}
> >
> >
> > Jakub
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] aarch64, v2: Fix up 2 other combine opt regressions vs. GCC8 [PR100075]
2021-04-19 11:16 ` Christophe Lyon
@ 2021-04-19 11:44 ` Jakub Jelinek
0 siblings, 0 replies; 7+ messages in thread
From: Jakub Jelinek @ 2021-04-19 11:44 UTC (permalink / raw)
To: Christophe Lyon
Cc: Richard Sandiford, Richard Earnshaw, Marcus Shawcroft,
Kyrylo Tkachov, gcc Patches
On Mon, Apr 19, 2021 at 01:16:13PM +0200, Christophe Lyon via Gcc-patches wrote:
> > > Here is an updated patch that I've also successfully bootstrapped/regtested
> > > on aarch64-linux.
> > >
> > > 2021-04-16 Jakub Jelinek <jakub@redhat.com>
> > >
> > > PR target/100075
> > > * config/aarch64/aarch64.md (*neg_asr_si2_extr, *extrsi5_insn_di): New
> > > define_insn patterns.
> > >
> > > * gcc.target/aarch64/pr100075.c: New test.
> >
> > OK, thanks.
> >
>
> Hi,
>
> The new test fails on aarch64_be, OK to add:
> +/* { dg-require-effective-target aarch64_little_endian } */
> ?
> The testcase looks really endianness dependent.
LGTM, thanks.
Jakub
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-04-19 11:44 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-15 15:43 [PATCH] aarch64: Fix up 2 other combine opt regressions vs. GCC8 [PR100075] Jakub Jelinek
2021-04-15 18:11 ` Richard Sandiford
2021-04-15 18:51 ` Jakub Jelinek
2021-04-16 7:38 ` [PATCH] aarch64, v2: " Jakub Jelinek
2021-04-16 11:29 ` Richard Sandiford
2021-04-19 11:16 ` Christophe Lyon
2021-04-19 11:44 ` Jakub Jelinek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).