* [PATCH] opcodes: fix wrong code in expand_binop_directly [PR117811]
@ 2025-03-21 9:47 Richard Earnshaw
2025-03-21 10:39 ` Richard Biener
2025-03-21 17:15 ` Andrew Pinski
0 siblings, 2 replies; 4+ messages in thread
From: Richard Earnshaw @ 2025-03-21 9:47 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Earnshaw, Richard Biener
If expand_binop_directly fails to add a REG_EQUAL note it tries to
unwind and restart. But it can unwind too far if expand_binop changed
some of the operands before calling it. We don't need to unwind that
far anyway since we should end up taking exactly the same route next
time, just without a target rtx.
To fix this we remove LAST from the argument list and let the callers
(all in expand_binop) do their own unwinding if the call fails.
Instead we unwind just as far as the entry to expand_binop_directly
and recurse within this function instead of all the way back up.
gcc/ChangeLog:
PR middle-end/117811
* optabs.cc (expand_binop_directly): Remove LAST as an argument,
instead record the last insn on entry. Only delete insns if
we need to restart and restart by calling ourself, not expand_binop.
(expand_binop): Update callers to expand_binop_directly. If it
fails to expand the operation, delete back to LAST.
gcc/testsuite:
PR middle-end/117811
* gcc.dg/torture/pr117811.c: New test.
---
gcc/optabs.cc | 24 +++++++++++-----------
gcc/testsuite/gcc.dg/torture/pr117811.c | 27 +++++++++++++++++++++++++
2 files changed, 39 insertions(+), 12 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/torture/pr117811.c
diff --git a/gcc/optabs.cc b/gcc/optabs.cc
index 36f2e6af8b5..0a14b1eef8a 100644
--- a/gcc/optabs.cc
+++ b/gcc/optabs.cc
@@ -1369,8 +1369,7 @@ avoid_expensive_constant (machine_mode mode, optab binoptab,
static rtx
expand_binop_directly (enum insn_code icode, machine_mode mode, optab binoptab,
rtx op0, rtx op1,
- rtx target, int unsignedp, enum optab_methods methods,
- rtx_insn *last)
+ rtx target, int unsignedp, enum optab_methods methods)
{
machine_mode xmode0 = insn_data[(int) icode].operand[1].mode;
machine_mode xmode1 = insn_data[(int) icode].operand[2].mode;
@@ -1380,6 +1379,7 @@ expand_binop_directly (enum insn_code icode, machine_mode mode, optab binoptab,
rtx_insn *pat;
rtx xop0 = op0, xop1 = op1;
bool canonicalize_op1 = false;
+ rtx_insn *last = get_last_insn ();
/* If it is a commutative operator and the modes would match
if we would swap the operands, we can save the conversions. */
@@ -1444,10 +1444,7 @@ expand_binop_directly (enum insn_code icode, machine_mode mode, optab binoptab,
tmp_mode = insn_data[(int) icode].operand[0].mode;
if (VECTOR_MODE_P (mode)
&& maybe_ne (GET_MODE_NUNITS (tmp_mode), 2 * GET_MODE_NUNITS (mode)))
- {
- delete_insns_since (last);
- return NULL_RTX;
- }
+ return NULL_RTX;
}
else
tmp_mode = mode;
@@ -1467,14 +1464,14 @@ expand_binop_directly (enum insn_code icode, machine_mode mode, optab binoptab,
ops[1].value, ops[2].value, mode0))
{
delete_insns_since (last);
- return expand_binop (mode, binoptab, op0, op1, NULL_RTX,
- unsignedp, methods);
+ return expand_binop_directly (icode, mode, binoptab, op0, op1,
+ NULL_RTX, unsignedp, methods);
}
emit_insn (pat);
return ops[0].value;
}
- delete_insns_since (last);
+
return NULL_RTX;
}
@@ -1543,9 +1540,10 @@ expand_binop (machine_mode mode, optab binoptab, rtx op0, rtx op1,
if (icode != CODE_FOR_nothing)
{
temp = expand_binop_directly (icode, mode, binoptab, op0, op1,
- target, unsignedp, methods, last);
+ target, unsignedp, methods);
if (temp)
return temp;
+ delete_insns_since (last);
}
}
@@ -1571,9 +1569,10 @@ expand_binop (machine_mode mode, optab binoptab, rtx op0, rtx op1,
NULL_RTX, unsignedp, OPTAB_DIRECT);
temp = expand_binop_directly (icode, int_mode, otheroptab, op0, newop1,
- target, unsignedp, methods, last);
+ target, unsignedp, methods);
if (temp)
return temp;
+ delete_insns_since (last);
}
/* If this is a multiply, see if we can do a widening operation that
@@ -1637,9 +1636,10 @@ expand_binop (machine_mode mode, optab binoptab, rtx op0, rtx op1,
if (vop1)
{
temp = expand_binop_directly (icode, mode, otheroptab, op0, vop1,
- target, unsignedp, methods, last);
+ target, unsignedp, methods);
if (temp)
return temp;
+ delete_insns_since (last);
}
}
}
diff --git a/gcc/testsuite/gcc.dg/torture/pr117811.c b/gcc/testsuite/gcc.dg/torture/pr117811.c
new file mode 100644
index 00000000000..13d7e134780
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr117811.c
@@ -0,0 +1,27 @@
+/* { dg-do run } */
+
+#include <string.h>
+
+typedef int v4 __attribute__((vector_size (4 * sizeof (int))));
+
+void __attribute__((noclone,noinline)) do_shift (v4 *vec, int shift)
+{
+ v4 t = *vec;
+
+ if (shift > 0)
+ {
+ t = t >> shift;
+ }
+
+ *vec = t;
+}
+
+int main ()
+{
+ v4 vec = {0x1000000, 0x2000, 0x300, 0x40};
+ v4 vec2 = {0x100000, 0x200, 0x30, 0x4};
+ do_shift (&vec, 4);
+ if (memcmp (&vec, &vec2, sizeof (v4)) != 0)
+ __builtin_abort ();
+ return 0;
+}
--
2.34.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] opcodes: fix wrong code in expand_binop_directly [PR117811]
2025-03-21 9:47 [PATCH] opcodes: fix wrong code in expand_binop_directly [PR117811] Richard Earnshaw
@ 2025-03-21 10:39 ` Richard Biener
2025-03-21 11:46 ` Christophe Lyon
2025-03-21 17:15 ` Andrew Pinski
1 sibling, 1 reply; 4+ messages in thread
From: Richard Biener @ 2025-03-21 10:39 UTC (permalink / raw)
To: Richard Earnshaw; +Cc: gcc-patches
On Fri, 21 Mar 2025, Richard Earnshaw wrote:
> If expand_binop_directly fails to add a REG_EQUAL note it tries to
> unwind and restart. But it can unwind too far if expand_binop changed
> some of the operands before calling it. We don't need to unwind that
> far anyway since we should end up taking exactly the same route next
> time, just without a target rtx.
>
> To fix this we remove LAST from the argument list and let the callers
> (all in expand_binop) do their own unwinding if the call fails.
> Instead we unwind just as far as the entry to expand_binop_directly
> and recurse within this function instead of all the way back up.
This looks good and like a nice cleanup as well. But please give
others more familiar with this code the chance to chime in.
Thanks,
Richard.
> gcc/ChangeLog:
>
> PR middle-end/117811
> * optabs.cc (expand_binop_directly): Remove LAST as an argument,
> instead record the last insn on entry. Only delete insns if
> we need to restart and restart by calling ourself, not expand_binop.
> (expand_binop): Update callers to expand_binop_directly. If it
> fails to expand the operation, delete back to LAST.
>
> gcc/testsuite:
>
> PR middle-end/117811
> * gcc.dg/torture/pr117811.c: New test.
> ---
> gcc/optabs.cc | 24 +++++++++++-----------
> gcc/testsuite/gcc.dg/torture/pr117811.c | 27 +++++++++++++++++++++++++
> 2 files changed, 39 insertions(+), 12 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/torture/pr117811.c
>
> diff --git a/gcc/optabs.cc b/gcc/optabs.cc
> index 36f2e6af8b5..0a14b1eef8a 100644
> --- a/gcc/optabs.cc
> +++ b/gcc/optabs.cc
> @@ -1369,8 +1369,7 @@ avoid_expensive_constant (machine_mode mode, optab binoptab,
> static rtx
> expand_binop_directly (enum insn_code icode, machine_mode mode, optab binoptab,
> rtx op0, rtx op1,
> - rtx target, int unsignedp, enum optab_methods methods,
> - rtx_insn *last)
> + rtx target, int unsignedp, enum optab_methods methods)
> {
> machine_mode xmode0 = insn_data[(int) icode].operand[1].mode;
> machine_mode xmode1 = insn_data[(int) icode].operand[2].mode;
> @@ -1380,6 +1379,7 @@ expand_binop_directly (enum insn_code icode, machine_mode mode, optab binoptab,
> rtx_insn *pat;
> rtx xop0 = op0, xop1 = op1;
> bool canonicalize_op1 = false;
> + rtx_insn *last = get_last_insn ();
>
> /* If it is a commutative operator and the modes would match
> if we would swap the operands, we can save the conversions. */
> @@ -1444,10 +1444,7 @@ expand_binop_directly (enum insn_code icode, machine_mode mode, optab binoptab,
> tmp_mode = insn_data[(int) icode].operand[0].mode;
> if (VECTOR_MODE_P (mode)
> && maybe_ne (GET_MODE_NUNITS (tmp_mode), 2 * GET_MODE_NUNITS (mode)))
> - {
> - delete_insns_since (last);
> - return NULL_RTX;
> - }
> + return NULL_RTX;
> }
> else
> tmp_mode = mode;
> @@ -1467,14 +1464,14 @@ expand_binop_directly (enum insn_code icode, machine_mode mode, optab binoptab,
> ops[1].value, ops[2].value, mode0))
> {
> delete_insns_since (last);
> - return expand_binop (mode, binoptab, op0, op1, NULL_RTX,
> - unsignedp, methods);
> + return expand_binop_directly (icode, mode, binoptab, op0, op1,
> + NULL_RTX, unsignedp, methods);
> }
>
> emit_insn (pat);
> return ops[0].value;
> }
> - delete_insns_since (last);
> +
> return NULL_RTX;
> }
>
> @@ -1543,9 +1540,10 @@ expand_binop (machine_mode mode, optab binoptab, rtx op0, rtx op1,
> if (icode != CODE_FOR_nothing)
> {
> temp = expand_binop_directly (icode, mode, binoptab, op0, op1,
> - target, unsignedp, methods, last);
> + target, unsignedp, methods);
> if (temp)
> return temp;
> + delete_insns_since (last);
> }
> }
>
> @@ -1571,9 +1569,10 @@ expand_binop (machine_mode mode, optab binoptab, rtx op0, rtx op1,
> NULL_RTX, unsignedp, OPTAB_DIRECT);
>
> temp = expand_binop_directly (icode, int_mode, otheroptab, op0, newop1,
> - target, unsignedp, methods, last);
> + target, unsignedp, methods);
> if (temp)
> return temp;
> + delete_insns_since (last);
> }
>
> /* If this is a multiply, see if we can do a widening operation that
> @@ -1637,9 +1636,10 @@ expand_binop (machine_mode mode, optab binoptab, rtx op0, rtx op1,
> if (vop1)
> {
> temp = expand_binop_directly (icode, mode, otheroptab, op0, vop1,
> - target, unsignedp, methods, last);
> + target, unsignedp, methods);
> if (temp)
> return temp;
> + delete_insns_since (last);
> }
> }
> }
> diff --git a/gcc/testsuite/gcc.dg/torture/pr117811.c b/gcc/testsuite/gcc.dg/torture/pr117811.c
> new file mode 100644
> index 00000000000..13d7e134780
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr117811.c
> @@ -0,0 +1,27 @@
> +/* { dg-do run } */
> +
> +#include <string.h>
> +
> +typedef int v4 __attribute__((vector_size (4 * sizeof (int))));
> +
> +void __attribute__((noclone,noinline)) do_shift (v4 *vec, int shift)
> +{
> + v4 t = *vec;
> +
> + if (shift > 0)
> + {
> + t = t >> shift;
> + }
> +
> + *vec = t;
> +}
> +
> +int main ()
> +{
> + v4 vec = {0x1000000, 0x2000, 0x300, 0x40};
> + v4 vec2 = {0x100000, 0x200, 0x30, 0x4};
> + do_shift (&vec, 4);
> + if (memcmp (&vec, &vec2, sizeof (v4)) != 0)
> + __builtin_abort ();
> + return 0;
> +}
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] opcodes: fix wrong code in expand_binop_directly [PR117811]
2025-03-21 10:39 ` Richard Biener
@ 2025-03-21 11:46 ` Christophe Lyon
0 siblings, 0 replies; 4+ messages in thread
From: Christophe Lyon @ 2025-03-21 11:46 UTC (permalink / raw)
To: Richard Biener; +Cc: Richard Earnshaw, gcc-patches
On Fri, 21 Mar 2025 at 11:39, Richard Biener <rguenther@suse.de> wrote:
>
> On Fri, 21 Mar 2025, Richard Earnshaw wrote:
>
> > If expand_binop_directly fails to add a REG_EQUAL note it tries to
> > unwind and restart. But it can unwind too far if expand_binop changed
> > some of the operands before calling it. We don't need to unwind that
> > far anyway since we should end up taking exactly the same route next
> > time, just without a target rtx.
> >
> > To fix this we remove LAST from the argument list and let the callers
> > (all in expand_binop) do their own unwinding if the call fails.
> > Instead we unwind just as far as the entry to expand_binop_directly
> > and recurse within this function instead of all the way back up.
>
> This looks good and like a nice cleanup as well. But please give
> others more familiar with this code the chance to chime in.
>
I was running a bootstrap on arm-linux-gnueabihf with the patch Andrew
attached in bugzilla. It passes bootstrap and make check, I think it
applies to this patch as well, as it is very similar.
Thanks,
Christophe
> Thanks,
> Richard.
>
> > gcc/ChangeLog:
> >
> > PR middle-end/117811
> > * optabs.cc (expand_binop_directly): Remove LAST as an argument,
> > instead record the last insn on entry. Only delete insns if
> > we need to restart and restart by calling ourself, not expand_binop.
> > (expand_binop): Update callers to expand_binop_directly. If it
> > fails to expand the operation, delete back to LAST.
> >
> > gcc/testsuite:
> >
> > PR middle-end/117811
> > * gcc.dg/torture/pr117811.c: New test.
> > ---
> > gcc/optabs.cc | 24 +++++++++++-----------
> > gcc/testsuite/gcc.dg/torture/pr117811.c | 27 +++++++++++++++++++++++++
> > 2 files changed, 39 insertions(+), 12 deletions(-)
> > create mode 100644 gcc/testsuite/gcc.dg/torture/pr117811.c
> >
> > diff --git a/gcc/optabs.cc b/gcc/optabs.cc
> > index 36f2e6af8b5..0a14b1eef8a 100644
> > --- a/gcc/optabs.cc
> > +++ b/gcc/optabs.cc
> > @@ -1369,8 +1369,7 @@ avoid_expensive_constant (machine_mode mode, optab binoptab,
> > static rtx
> > expand_binop_directly (enum insn_code icode, machine_mode mode, optab binoptab,
> > rtx op0, rtx op1,
> > - rtx target, int unsignedp, enum optab_methods methods,
> > - rtx_insn *last)
> > + rtx target, int unsignedp, enum optab_methods methods)
> > {
> > machine_mode xmode0 = insn_data[(int) icode].operand[1].mode;
> > machine_mode xmode1 = insn_data[(int) icode].operand[2].mode;
> > @@ -1380,6 +1379,7 @@ expand_binop_directly (enum insn_code icode, machine_mode mode, optab binoptab,
> > rtx_insn *pat;
> > rtx xop0 = op0, xop1 = op1;
> > bool canonicalize_op1 = false;
> > + rtx_insn *last = get_last_insn ();
> >
> > /* If it is a commutative operator and the modes would match
> > if we would swap the operands, we can save the conversions. */
> > @@ -1444,10 +1444,7 @@ expand_binop_directly (enum insn_code icode, machine_mode mode, optab binoptab,
> > tmp_mode = insn_data[(int) icode].operand[0].mode;
> > if (VECTOR_MODE_P (mode)
> > && maybe_ne (GET_MODE_NUNITS (tmp_mode), 2 * GET_MODE_NUNITS (mode)))
> > - {
> > - delete_insns_since (last);
> > - return NULL_RTX;
> > - }
> > + return NULL_RTX;
> > }
> > else
> > tmp_mode = mode;
> > @@ -1467,14 +1464,14 @@ expand_binop_directly (enum insn_code icode, machine_mode mode, optab binoptab,
> > ops[1].value, ops[2].value, mode0))
> > {
> > delete_insns_since (last);
> > - return expand_binop (mode, binoptab, op0, op1, NULL_RTX,
> > - unsignedp, methods);
> > + return expand_binop_directly (icode, mode, binoptab, op0, op1,
> > + NULL_RTX, unsignedp, methods);
> > }
> >
> > emit_insn (pat);
> > return ops[0].value;
> > }
> > - delete_insns_since (last);
> > +
> > return NULL_RTX;
> > }
> >
> > @@ -1543,9 +1540,10 @@ expand_binop (machine_mode mode, optab binoptab, rtx op0, rtx op1,
> > if (icode != CODE_FOR_nothing)
> > {
> > temp = expand_binop_directly (icode, mode, binoptab, op0, op1,
> > - target, unsignedp, methods, last);
> > + target, unsignedp, methods);
> > if (temp)
> > return temp;
> > + delete_insns_since (last);
> > }
> > }
> >
> > @@ -1571,9 +1569,10 @@ expand_binop (machine_mode mode, optab binoptab, rtx op0, rtx op1,
> > NULL_RTX, unsignedp, OPTAB_DIRECT);
> >
> > temp = expand_binop_directly (icode, int_mode, otheroptab, op0, newop1,
> > - target, unsignedp, methods, last);
> > + target, unsignedp, methods);
> > if (temp)
> > return temp;
> > + delete_insns_since (last);
> > }
> >
> > /* If this is a multiply, see if we can do a widening operation that
> > @@ -1637,9 +1636,10 @@ expand_binop (machine_mode mode, optab binoptab, rtx op0, rtx op1,
> > if (vop1)
> > {
> > temp = expand_binop_directly (icode, mode, otheroptab, op0, vop1,
> > - target, unsignedp, methods, last);
> > + target, unsignedp, methods);
> > if (temp)
> > return temp;
> > + delete_insns_since (last);
> > }
> > }
> > }
> > diff --git a/gcc/testsuite/gcc.dg/torture/pr117811.c b/gcc/testsuite/gcc.dg/torture/pr117811.c
> > new file mode 100644
> > index 00000000000..13d7e134780
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/torture/pr117811.c
> > @@ -0,0 +1,27 @@
> > +/* { dg-do run } */
> > +
> > +#include <string.h>
> > +
> > +typedef int v4 __attribute__((vector_size (4 * sizeof (int))));
> > +
> > +void __attribute__((noclone,noinline)) do_shift (v4 *vec, int shift)
> > +{
> > + v4 t = *vec;
> > +
> > + if (shift > 0)
> > + {
> > + t = t >> shift;
> > + }
> > +
> > + *vec = t;
> > +}
> > +
> > +int main ()
> > +{
> > + v4 vec = {0x1000000, 0x2000, 0x300, 0x40};
> > + v4 vec2 = {0x100000, 0x200, 0x30, 0x4};
> > + do_shift (&vec, 4);
> > + if (memcmp (&vec, &vec2, sizeof (v4)) != 0)
> > + __builtin_abort ();
> > + return 0;
> > +}
> >
>
> --
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH,
> Frankenstrasse 146, 90461 Nuernberg, Germany;
> GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] opcodes: fix wrong code in expand_binop_directly [PR117811]
2025-03-21 9:47 [PATCH] opcodes: fix wrong code in expand_binop_directly [PR117811] Richard Earnshaw
2025-03-21 10:39 ` Richard Biener
@ 2025-03-21 17:15 ` Andrew Pinski
1 sibling, 0 replies; 4+ messages in thread
From: Andrew Pinski @ 2025-03-21 17:15 UTC (permalink / raw)
To: Richard Earnshaw; +Cc: gcc-patches, Richard Biener
On Fri, Mar 21, 2025 at 2:51 AM Richard Earnshaw <rearnsha@arm.com> wrote:
>
> If expand_binop_directly fails to add a REG_EQUAL note it tries to
> unwind and restart. But it can unwind too far if expand_binop changed
> some of the operands before calling it. We don't need to unwind that
> far anyway since we should end up taking exactly the same route next
> time, just without a target rtx.
>
> To fix this we remove LAST from the argument list and let the callers
> (all in expand_binop) do their own unwinding if the call fails.
> Instead we unwind just as far as the entry to expand_binop_directly
> and recurse within this function instead of all the way back up.
>
> gcc/ChangeLog:
>
> PR middle-end/117811
> * optabs.cc (expand_binop_directly): Remove LAST as an argument,
> instead record the last insn on entry. Only delete insns if
> we need to restart and restart by calling ourself, not expand_binop.
> (expand_binop): Update callers to expand_binop_directly. If it
> fails to expand the operation, delete back to LAST.
This looks better than the patch I had came up with. Thanks again for
taking up the fix to finish..
Thanks,
Andrew
>
> gcc/testsuite:
>
> PR middle-end/117811
> * gcc.dg/torture/pr117811.c: New test.
> ---
> gcc/optabs.cc | 24 +++++++++++-----------
> gcc/testsuite/gcc.dg/torture/pr117811.c | 27 +++++++++++++++++++++++++
> 2 files changed, 39 insertions(+), 12 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/torture/pr117811.c
>
> diff --git a/gcc/optabs.cc b/gcc/optabs.cc
> index 36f2e6af8b5..0a14b1eef8a 100644
> --- a/gcc/optabs.cc
> +++ b/gcc/optabs.cc
> @@ -1369,8 +1369,7 @@ avoid_expensive_constant (machine_mode mode, optab binoptab,
> static rtx
> expand_binop_directly (enum insn_code icode, machine_mode mode, optab binoptab,
> rtx op0, rtx op1,
> - rtx target, int unsignedp, enum optab_methods methods,
> - rtx_insn *last)
> + rtx target, int unsignedp, enum optab_methods methods)
> {
> machine_mode xmode0 = insn_data[(int) icode].operand[1].mode;
> machine_mode xmode1 = insn_data[(int) icode].operand[2].mode;
> @@ -1380,6 +1379,7 @@ expand_binop_directly (enum insn_code icode, machine_mode mode, optab binoptab,
> rtx_insn *pat;
> rtx xop0 = op0, xop1 = op1;
> bool canonicalize_op1 = false;
> + rtx_insn *last = get_last_insn ();
>
> /* If it is a commutative operator and the modes would match
> if we would swap the operands, we can save the conversions. */
> @@ -1444,10 +1444,7 @@ expand_binop_directly (enum insn_code icode, machine_mode mode, optab binoptab,
> tmp_mode = insn_data[(int) icode].operand[0].mode;
> if (VECTOR_MODE_P (mode)
> && maybe_ne (GET_MODE_NUNITS (tmp_mode), 2 * GET_MODE_NUNITS (mode)))
> - {
> - delete_insns_since (last);
> - return NULL_RTX;
> - }
> + return NULL_RTX;
> }
> else
> tmp_mode = mode;
> @@ -1467,14 +1464,14 @@ expand_binop_directly (enum insn_code icode, machine_mode mode, optab binoptab,
> ops[1].value, ops[2].value, mode0))
> {
> delete_insns_since (last);
> - return expand_binop (mode, binoptab, op0, op1, NULL_RTX,
> - unsignedp, methods);
> + return expand_binop_directly (icode, mode, binoptab, op0, op1,
> + NULL_RTX, unsignedp, methods);
> }
>
> emit_insn (pat);
> return ops[0].value;
> }
> - delete_insns_since (last);
> +
> return NULL_RTX;
> }
>
> @@ -1543,9 +1540,10 @@ expand_binop (machine_mode mode, optab binoptab, rtx op0, rtx op1,
> if (icode != CODE_FOR_nothing)
> {
> temp = expand_binop_directly (icode, mode, binoptab, op0, op1,
> - target, unsignedp, methods, last);
> + target, unsignedp, methods);
> if (temp)
> return temp;
> + delete_insns_since (last);
> }
> }
>
> @@ -1571,9 +1569,10 @@ expand_binop (machine_mode mode, optab binoptab, rtx op0, rtx op1,
> NULL_RTX, unsignedp, OPTAB_DIRECT);
>
> temp = expand_binop_directly (icode, int_mode, otheroptab, op0, newop1,
> - target, unsignedp, methods, last);
> + target, unsignedp, methods);
> if (temp)
> return temp;
> + delete_insns_since (last);
> }
>
> /* If this is a multiply, see if we can do a widening operation that
> @@ -1637,9 +1636,10 @@ expand_binop (machine_mode mode, optab binoptab, rtx op0, rtx op1,
> if (vop1)
> {
> temp = expand_binop_directly (icode, mode, otheroptab, op0, vop1,
> - target, unsignedp, methods, last);
> + target, unsignedp, methods);
> if (temp)
> return temp;
> + delete_insns_since (last);
> }
> }
> }
> diff --git a/gcc/testsuite/gcc.dg/torture/pr117811.c b/gcc/testsuite/gcc.dg/torture/pr117811.c
> new file mode 100644
> index 00000000000..13d7e134780
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr117811.c
> @@ -0,0 +1,27 @@
> +/* { dg-do run } */
> +
> +#include <string.h>
> +
> +typedef int v4 __attribute__((vector_size (4 * sizeof (int))));
> +
> +void __attribute__((noclone,noinline)) do_shift (v4 *vec, int shift)
> +{
> + v4 t = *vec;
> +
> + if (shift > 0)
> + {
> + t = t >> shift;
> + }
> +
> + *vec = t;
> +}
> +
> +int main ()
> +{
> + v4 vec = {0x1000000, 0x2000, 0x300, 0x40};
> + v4 vec2 = {0x100000, 0x200, 0x30, 0x4};
> + do_shift (&vec, 4);
> + if (memcmp (&vec, &vec2, sizeof (v4)) != 0)
> + __builtin_abort ();
> + return 0;
> +}
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-03-21 17:15 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-03-21 9:47 [PATCH] opcodes: fix wrong code in expand_binop_directly [PR117811] Richard Earnshaw
2025-03-21 10:39 ` Richard Biener
2025-03-21 11:46 ` Christophe Lyon
2025-03-21 17:15 ` Andrew Pinski
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).