* [PATCH] i386: Fix up *_doubleword_mask [PR105825]
@ 2022-06-03 10:17 Jakub Jelinek
2022-06-03 10:23 ` Uros Bizjak
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2022-06-03 10:17 UTC (permalink / raw)
To: Uros Bizjak; +Cc: gcc-patches
Hi!
My PR105778 patch apparently broke the following testcase.
If the mask has the top relevant bit clear (i.e. we know we are shifting
by 0 to wordsize bits - 1) but doesn't have all the bits below it set,
we emit andsi3 before the shift sequence. When the pattern had :SI
for that operand, that was just fine, but now that it can be also HImode
or for -m64 DImode, we either can use a lowpart or paradoxical subreg to
SImode as the following patch, or we could use a HImode or DImode AND.
Ok for trunk if it passes bootstrap/regtest on x86_64-linux and i686-linux?
2022-06-03 Jakub Jelinek <jakub@redhat.com>
PR target/105825
* config/i386/i386.md (*ashl<dwi>3_doubleword_mask,
*<insn><dwi>3_doubleword_mask): If top bit of mask is clear, but lower
bits of mask aren't all set and operands[2] doesn't have SImode,
force it to get and use subreg to SImode for andsi3 operand.
* gcc.dg/pr105825.c: New test.
--- gcc/config/i386/i386.md.jj 2022-06-02 10:40:00.034660893 +0200
+++ gcc/config/i386/i386.md 2022-06-03 12:00:39.323292767 +0200
@@ -11935,6 +11935,11 @@ (define_insn_and_split "*ashl<dwi>3_doub
!= ((<MODE_SIZE> * BITS_PER_UNIT) - 1))
{
rtx tem = gen_reg_rtx (SImode);
+ if (GET_MODE (operands[2]) != SImode)
+ {
+ operands[2] = force_reg (GET_MODE (operands[2]), operands[2]);
+ operands[2] = gen_lowpart (SImode, operands[2]);
+ }
emit_insn (gen_andsi3 (tem, operands[2], operands[3]));
operands[2] = tem;
}
@@ -12900,6 +12905,11 @@ (define_insn_and_split "*<insn><dwi>3_do
!= ((<MODE_SIZE> * BITS_PER_UNIT) - 1))
{
rtx tem = gen_reg_rtx (SImode);
+ if (GET_MODE (operands[2]) != SImode)
+ {
+ operands[2] = force_reg (GET_MODE (operands[2]), operands[2]);
+ operands[2] = gen_lowpart (SImode, operands[2]);
+ }
emit_insn (gen_andsi3 (tem, operands[2], operands[3]));
operands[2] = tem;
}
--- gcc/testsuite/gcc.dg/pr105825.c.jj 2022-06-03 12:01:58.008460659 +0200
+++ gcc/testsuite/gcc.dg/pr105825.c 2022-06-03 12:01:41.259637783 +0200
@@ -0,0 +1,13 @@
+/* PR target/105825 */
+/* { dg-do compile { target int128 } } */
+/* { dg-options "-O2" } */
+/* { dg-additional-options "-mavx" { target avx } } */
+
+__int128 j;
+int i;
+
+void
+foo (void)
+{
+ j <<= __builtin_parityll (i);
+}
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] i386: Fix up *_doubleword_mask [PR105825]
2022-06-03 10:17 [PATCH] i386: Fix up *_doubleword_mask [PR105825] Jakub Jelinek
@ 2022-06-03 10:23 ` Uros Bizjak
2022-06-03 10:38 ` Jakub Jelinek
0 siblings, 1 reply; 4+ messages in thread
From: Uros Bizjak @ 2022-06-03 10:23 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Fri, Jun 3, 2022 at 12:17 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> My PR105778 patch apparently broke the following testcase.
> If the mask has the top relevant bit clear (i.e. we know we are shifting
> by 0 to wordsize bits - 1) but doesn't have all the bits below it set,
> we emit andsi3 before the shift sequence. When the pattern had :SI
> for that operand, that was just fine, but now that it can be also HImode
> or for -m64 DImode, we either can use a lowpart or paradoxical subreg to
> SImode as the following patch, or we could use a HImode or DImode AND.
I think it is better to leave the operation in its natural mode and
leave the peephole pass to do its magic, depending on the target.
Uros.
> Ok for trunk if it passes bootstrap/regtest on x86_64-linux and i686-linux?
>
> 2022-06-03 Jakub Jelinek <jakub@redhat.com>
>
> PR target/105825
> * config/i386/i386.md (*ashl<dwi>3_doubleword_mask,
> *<insn><dwi>3_doubleword_mask): If top bit of mask is clear, but lower
> bits of mask aren't all set and operands[2] doesn't have SImode,
> force it to get and use subreg to SImode for andsi3 operand.
>
> * gcc.dg/pr105825.c: New test.
>
> --- gcc/config/i386/i386.md.jj 2022-06-02 10:40:00.034660893 +0200
> +++ gcc/config/i386/i386.md 2022-06-03 12:00:39.323292767 +0200
> @@ -11935,6 +11935,11 @@ (define_insn_and_split "*ashl<dwi>3_doub
> != ((<MODE_SIZE> * BITS_PER_UNIT) - 1))
> {
> rtx tem = gen_reg_rtx (SImode);
> + if (GET_MODE (operands[2]) != SImode)
> + {
> + operands[2] = force_reg (GET_MODE (operands[2]), operands[2]);
> + operands[2] = gen_lowpart (SImode, operands[2]);
> + }
> emit_insn (gen_andsi3 (tem, operands[2], operands[3]));
> operands[2] = tem;
> }
> @@ -12900,6 +12905,11 @@ (define_insn_and_split "*<insn><dwi>3_do
> != ((<MODE_SIZE> * BITS_PER_UNIT) - 1))
> {
> rtx tem = gen_reg_rtx (SImode);
> + if (GET_MODE (operands[2]) != SImode)
> + {
> + operands[2] = force_reg (GET_MODE (operands[2]), operands[2]);
> + operands[2] = gen_lowpart (SImode, operands[2]);
> + }
> emit_insn (gen_andsi3 (tem, operands[2], operands[3]));
> operands[2] = tem;
> }
> --- gcc/testsuite/gcc.dg/pr105825.c.jj 2022-06-03 12:01:58.008460659 +0200
> +++ gcc/testsuite/gcc.dg/pr105825.c 2022-06-03 12:01:41.259637783 +0200
> @@ -0,0 +1,13 @@
> +/* PR target/105825 */
> +/* { dg-do compile { target int128 } } */
> +/* { dg-options "-O2" } */
> +/* { dg-additional-options "-mavx" { target avx } } */
> +
> +__int128 j;
> +int i;
> +
> +void
> +foo (void)
> +{
> + j <<= __builtin_parityll (i);
> +}
>
> Jakub
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] i386: Fix up *_doubleword_mask [PR105825]
2022-06-03 10:23 ` Uros Bizjak
@ 2022-06-03 10:38 ` Jakub Jelinek
2022-06-03 11:10 ` Uros Bizjak
0 siblings, 1 reply; 4+ messages in thread
From: Jakub Jelinek @ 2022-06-03 10:38 UTC (permalink / raw)
To: Uros Bizjak; +Cc: gcc-patches
On Fri, Jun 03, 2022 at 12:23:36PM +0200, Uros Bizjak wrote:
> I think it is better to leave the operation in its natural mode and
> leave the peephole pass to do its magic, depending on the target.
So like this?
2022-06-03 Jakub Jelinek <jakub@redhat.com>
PR target/105825
* config/i386/i386.md (*ashl<dwi>3_doubleword_mask,
*<insn><dwi>3_doubleword_mask): If top bit of mask is clear, but lower
bits of mask aren't all set, use operands[2] mode for the AND
operation instead of always SImode.
* gcc.dg/pr105825.c: New test.
--- gcc/config/i386/i386.md.jj 2022-06-02 10:40:00.034660893 +0200
+++ gcc/config/i386/i386.md 2022-06-03 12:33:38.180448918 +0200
@@ -11934,8 +11934,16 @@ (define_insn_and_split "*ashl<dwi>3_doub
if ((INTVAL (operands[3]) & ((<MODE_SIZE> * BITS_PER_UNIT) - 1))
!= ((<MODE_SIZE> * BITS_PER_UNIT) - 1))
{
- rtx tem = gen_reg_rtx (SImode);
- emit_insn (gen_andsi3 (tem, operands[2], operands[3]));
+ rtx tem = gen_reg_rtx (GET_MODE (operands[2]));
+ rtx (*gen) (rtx, rtx, rtx);
+ switch (GET_MODE (operands[2]))
+ {
+ case E_HImode: gen = gen_andhi3; break;
+ case E_SImode: gen = gen_andsi3; break;
+ case E_DImode: gen = gen_anddi3; break;
+ default: gcc_unreachable ();
+ }
+ emit_insn (gen (tem, operands[2], operands[3]));
operands[2] = tem;
}
@@ -12899,8 +12907,16 @@ (define_insn_and_split "*<insn><dwi>3_do
if ((INTVAL (operands[3]) & ((<MODE_SIZE> * BITS_PER_UNIT) - 1))
!= ((<MODE_SIZE> * BITS_PER_UNIT) - 1))
{
- rtx tem = gen_reg_rtx (SImode);
- emit_insn (gen_andsi3 (tem, operands[2], operands[3]));
+ rtx tem = gen_reg_rtx (GET_MODE (operands[2]));
+ rtx (*gen) (rtx, rtx, rtx);
+ switch (GET_MODE (operands[2]))
+ {
+ case E_HImode: gen = gen_andhi3; break;
+ case E_SImode: gen = gen_andsi3; break;
+ case E_DImode: gen = gen_anddi3; break;
+ default: gcc_unreachable ();
+ }
+ emit_insn (gen (tem, operands[2], operands[3]));
operands[2] = tem;
}
--- gcc/testsuite/gcc.dg/pr105825.c.jj 2022-06-03 12:01:58.008460659 +0200
+++ gcc/testsuite/gcc.dg/pr105825.c 2022-06-03 12:01:41.259637783 +0200
@@ -0,0 +1,13 @@
+/* PR target/105825 */
+/* { dg-do compile { target int128 } } */
+/* { dg-options "-O2" } */
+/* { dg-additional-options "-mavx" { target avx } } */
+
+__int128 j;
+int i;
+
+void
+foo (void)
+{
+ j <<= __builtin_parityll (i);
+}
Jakub
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] i386: Fix up *_doubleword_mask [PR105825]
2022-06-03 10:38 ` Jakub Jelinek
@ 2022-06-03 11:10 ` Uros Bizjak
0 siblings, 0 replies; 4+ messages in thread
From: Uros Bizjak @ 2022-06-03 11:10 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Fri, Jun 3, 2022 at 12:38 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Fri, Jun 03, 2022 at 12:23:36PM +0200, Uros Bizjak wrote:
> > I think it is better to leave the operation in its natural mode and
> > leave the peephole pass to do its magic, depending on the target.
>
> So like this?
You can use ix86_expand_binary_operator.
> 2022-06-03 Jakub Jelinek <jakub@redhat.com>
>
> PR target/105825
> * config/i386/i386.md (*ashl<dwi>3_doubleword_mask,
> *<insn><dwi>3_doubleword_mask): If top bit of mask is clear, but lower
> bits of mask aren't all set, use operands[2] mode for the AND
> operation instead of always SImode.
>
> * gcc.dg/pr105825.c: New test.
OK with using ix86_expand_binary_operator.
Thanks,
Uros.
>
> --- gcc/config/i386/i386.md.jj 2022-06-02 10:40:00.034660893 +0200
> +++ gcc/config/i386/i386.md 2022-06-03 12:33:38.180448918 +0200
> @@ -11934,8 +11934,16 @@ (define_insn_and_split "*ashl<dwi>3_doub
> if ((INTVAL (operands[3]) & ((<MODE_SIZE> * BITS_PER_UNIT) - 1))
> != ((<MODE_SIZE> * BITS_PER_UNIT) - 1))
> {
> - rtx tem = gen_reg_rtx (SImode);
> - emit_insn (gen_andsi3 (tem, operands[2], operands[3]));
> + rtx tem = gen_reg_rtx (GET_MODE (operands[2]));
> + rtx (*gen) (rtx, rtx, rtx);
> + switch (GET_MODE (operands[2]))
> + {
> + case E_HImode: gen = gen_andhi3; break;
> + case E_SImode: gen = gen_andsi3; break;
> + case E_DImode: gen = gen_anddi3; break;
> + default: gcc_unreachable ();
> + }
> + emit_insn (gen (tem, operands[2], operands[3]));
> operands[2] = tem;
> }
>
> @@ -12899,8 +12907,16 @@ (define_insn_and_split "*<insn><dwi>3_do
> if ((INTVAL (operands[3]) & ((<MODE_SIZE> * BITS_PER_UNIT) - 1))
> != ((<MODE_SIZE> * BITS_PER_UNIT) - 1))
> {
> - rtx tem = gen_reg_rtx (SImode);
> - emit_insn (gen_andsi3 (tem, operands[2], operands[3]));
> + rtx tem = gen_reg_rtx (GET_MODE (operands[2]));
> + rtx (*gen) (rtx, rtx, rtx);
> + switch (GET_MODE (operands[2]))
> + {
> + case E_HImode: gen = gen_andhi3; break;
> + case E_SImode: gen = gen_andsi3; break;
> + case E_DImode: gen = gen_anddi3; break;
> + default: gcc_unreachable ();
> + }
> + emit_insn (gen (tem, operands[2], operands[3]));
> operands[2] = tem;
> }
>
> --- gcc/testsuite/gcc.dg/pr105825.c.jj 2022-06-03 12:01:58.008460659 +0200
> +++ gcc/testsuite/gcc.dg/pr105825.c 2022-06-03 12:01:41.259637783 +0200
> @@ -0,0 +1,13 @@
> +/* PR target/105825 */
> +/* { dg-do compile { target int128 } } */
> +/* { dg-options "-O2" } */
> +/* { dg-additional-options "-mavx" { target avx } } */
> +
> +__int128 j;
> +int i;
> +
> +void
> +foo (void)
> +{
> + j <<= __builtin_parityll (i);
> +}
>
>
> Jakub
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-06-03 11:11 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-03 10:17 [PATCH] i386: Fix up *_doubleword_mask [PR105825] Jakub Jelinek
2022-06-03 10:23 ` Uros Bizjak
2022-06-03 10:38 ` Jakub Jelinek
2022-06-03 11:10 ` Uros Bizjak
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).