Fix PR 109919: ICE in emit_move_insn with some bit tests
Checks
Commit Message
The problem is I used expand_expr with the target but
we don't want to use the target here as it is the wrong
mode for the original expression. The testcase would ICE
deap down while trying to do a move to use the target.
Anyways just calling expand_expr with NULL_EXPR fixes
the issue.
Committed as obvious after a bootstrap/test on x86_64-linux-gnu.
PR middle-end/109919
gcc/ChangeLog:
* expr.cc (expand_single_bit_test): Don't use the
target for expand_expr.
gcc/testsuite/ChangeLog:
* gcc.c-torture/compile/pr109919-1.c: New test.
---
gcc/expr.cc | 2 +-
gcc/testsuite/gcc.c-torture/compile/pr109919-1.c | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr109919-1.c
Comments
On 5/20/23 19:09, Andrew Pinski via Gcc-patches wrote:
> The problem is I used expand_expr with the target but
> we don't want to use the target here as it is the wrong
> mode for the original expression. The testcase would ICE
> deap down while trying to do a move to use the target.
> Anyways just calling expand_expr with NULL_EXPR fixes
> the issue.
>
> Committed as obvious after a bootstrap/test on x86_64-linux-gnu.
>
> PR middle-end/109919
>
> gcc/ChangeLog:
>
> * expr.cc (expand_single_bit_test): Don't use the
> target for expand_expr.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.c-torture/compile/pr109919-1.c: New test.
Thanks. I'll respin the targets that failed. If you don't hear from
me, assume everything is happy again after this fix.
jeff
On Sat, May 20, 2023 at 6:26 PM Jeff Law via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
>
> On 5/20/23 19:09, Andrew Pinski via Gcc-patches wrote:
> > The problem is I used expand_expr with the target but
> > we don't want to use the target here as it is the wrong
> > mode for the original expression. The testcase would ICE
> > deap down while trying to do a move to use the target.
> > Anyways just calling expand_expr with NULL_EXPR fixes
> > the issue.
> >
> > Committed as obvious after a bootstrap/test on x86_64-linux-gnu.
> >
> > PR middle-end/109919
> >
> > gcc/ChangeLog:
> >
> > * expr.cc (expand_single_bit_test): Don't use the
> > target for expand_expr.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * gcc.c-torture/compile/pr109919-1.c: New test.
> Thanks. I'll respin the targets that failed. If you don't hear from
> me, assume everything is happy again after this fix.
Oh, I am going to test on aarch64-linux-gnu too just in case.
Expand is definitely something which I am not used to working on so I
figured I had made a mistake somewhere. I suspect I still made a
similar mistake later on too.
Thanks,
Andrew
> jeff
On 5/20/23 21:05, Andrew Pinski wrote:
> On Sat, May 20, 2023 at 6:26 PM Jeff Law via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>>
>>
>> On 5/20/23 19:09, Andrew Pinski via Gcc-patches wrote:
>>> The problem is I used expand_expr with the target but
>>> we don't want to use the target here as it is the wrong
>>> mode for the original expression. The testcase would ICE
>>> deap down while trying to do a move to use the target.
>>> Anyways just calling expand_expr with NULL_EXPR fixes
>>> the issue.
>>>
>>> Committed as obvious after a bootstrap/test on x86_64-linux-gnu.
>>>
>>> PR middle-end/109919
>>>
>>> gcc/ChangeLog:
>>>
>>> * expr.cc (expand_single_bit_test): Don't use the
>>> target for expand_expr.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> * gcc.c-torture/compile/pr109919-1.c: New test.
>> Thanks. I'll respin the targets that failed. If you don't hear from
>> me, assume everything is happy again after this fix.
>
> Oh, I am going to test on aarch64-linux-gnu too just in case.
> Expand is definitely something which I am not used to working on so I
> figured I had made a mistake somewhere. I suspect I still made a
> similar mistake later on too.
I'm seeing some execution failures. Building H8 bits now to debug as
it's the target I'm most familiar with. More info as it's available.
jeff
On Sat, May 20, 2023 at 8:25 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
>
>
>
> On 5/20/23 21:05, Andrew Pinski wrote:
> > On Sat, May 20, 2023 at 6:26 PM Jeff Law via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >>
> >>
> >> On 5/20/23 19:09, Andrew Pinski via Gcc-patches wrote:
> >>> The problem is I used expand_expr with the target but
> >>> we don't want to use the target here as it is the wrong
> >>> mode for the original expression. The testcase would ICE
> >>> deap down while trying to do a move to use the target.
> >>> Anyways just calling expand_expr with NULL_EXPR fixes
> >>> the issue.
> >>>
> >>> Committed as obvious after a bootstrap/test on x86_64-linux-gnu.
> >>>
> >>> PR middle-end/109919
> >>>
> >>> gcc/ChangeLog:
> >>>
> >>> * expr.cc (expand_single_bit_test): Don't use the
> >>> target for expand_expr.
> >>>
> >>> gcc/testsuite/ChangeLog:
> >>>
> >>> * gcc.c-torture/compile/pr109919-1.c: New test.
> >> Thanks. I'll respin the targets that failed. If you don't hear from
> >> me, assume everything is happy again after this fix.
> >
> > Oh, I am going to test on aarch64-linux-gnu too just in case.
> > Expand is definitely something which I am not used to working on so I
> > figured I had made a mistake somewhere. I suspect I still made a
> > similar mistake later on too.
> I'm seeing some execution failures. Building H8 bits now to debug as
> it's the target I'm most familiar with. More info as it's available.
Is H8 big-endian? I could have messed that up.
Thanks,
Andrew Pinski
>
> jeff
On Sat, May 20, 2023 at 8:28 PM Andrew Pinski <pinskia@gmail.com> wrote:
>
> On Sat, May 20, 2023 at 8:25 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
> >
> >
> >
> > On 5/20/23 21:05, Andrew Pinski wrote:
> > > On Sat, May 20, 2023 at 6:26 PM Jeff Law via Gcc-patches
> > > <gcc-patches@gcc.gnu.org> wrote:
> > >>
> > >>
> > >>
> > >> On 5/20/23 19:09, Andrew Pinski via Gcc-patches wrote:
> > >>> The problem is I used expand_expr with the target but
> > >>> we don't want to use the target here as it is the wrong
> > >>> mode for the original expression. The testcase would ICE
> > >>> deap down while trying to do a move to use the target.
> > >>> Anyways just calling expand_expr with NULL_EXPR fixes
> > >>> the issue.
> > >>>
> > >>> Committed as obvious after a bootstrap/test on x86_64-linux-gnu.
> > >>>
> > >>> PR middle-end/109919
> > >>>
> > >>> gcc/ChangeLog:
> > >>>
> > >>> * expr.cc (expand_single_bit_test): Don't use the
> > >>> target for expand_expr.
> > >>>
> > >>> gcc/testsuite/ChangeLog:
> > >>>
> > >>> * gcc.c-torture/compile/pr109919-1.c: New test.
> > >> Thanks. I'll respin the targets that failed. If you don't hear from
> > >> me, assume everything is happy again after this fix.
> > >
> > > Oh, I am going to test on aarch64-linux-gnu too just in case.
> > > Expand is definitely something which I am not used to working on so I
> > > figured I had made a mistake somewhere. I suspect I still made a
> > > similar mistake later on too.
> > I'm seeing some execution failures. Building H8 bits now to debug as
> > it's the target I'm most familiar with. More info as it's available.
>
> Is H8 big-endian? I could have messed that up.
If so, try the attached patch. I thought extract_bit_field's bitnum
field was like a shift and not like how BIT_FIELD_REF is defined.
Thanks,
Andrew
>
> Thanks,
> Andrew Pinski
>
> >
> > jeff
diff --git a/gcc/expr.cc b/gcc/expr.cc
index 02f24c00148..c033761f317 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -12958,7 +12958,12 @@ expand_single_bit_test (location_t loc, enum tree_code code,
rtx inner0 = expand_expr (inner, NULL_RTX, VOIDmode, EXPAND_NORMAL);
- inner0 = extract_bit_field (inner0, 1, bitnum, 1, target,
+ int bitpos = bitnum;
+
+ if (BYTES_BIG_ENDIAN)
+ bitpos = GET_MODE_BITSIZE (inner0) - 1 - bitpos;
+
+ inner0 = extract_bit_field (inner0, 1, bitpos, 1, target,
operand_mode, mode, 0, NULL);
if (code == EQ_EXPR)
On Sat, May 20, 2023 at 8:32 PM Andrew Pinski <pinskia@gmail.com> wrote:
>
> On Sat, May 20, 2023 at 8:28 PM Andrew Pinski <pinskia@gmail.com> wrote:
> >
> > On Sat, May 20, 2023 at 8:25 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
> > >
> > >
> > >
> > > On 5/20/23 21:05, Andrew Pinski wrote:
> > > > On Sat, May 20, 2023 at 6:26 PM Jeff Law via Gcc-patches
> > > > <gcc-patches@gcc.gnu.org> wrote:
> > > >>
> > > >>
> > > >>
> > > >> On 5/20/23 19:09, Andrew Pinski via Gcc-patches wrote:
> > > >>> The problem is I used expand_expr with the target but
> > > >>> we don't want to use the target here as it is the wrong
> > > >>> mode for the original expression. The testcase would ICE
> > > >>> deap down while trying to do a move to use the target.
> > > >>> Anyways just calling expand_expr with NULL_EXPR fixes
> > > >>> the issue.
> > > >>>
> > > >>> Committed as obvious after a bootstrap/test on x86_64-linux-gnu.
> > > >>>
> > > >>> PR middle-end/109919
> > > >>>
> > > >>> gcc/ChangeLog:
> > > >>>
> > > >>> * expr.cc (expand_single_bit_test): Don't use the
> > > >>> target for expand_expr.
> > > >>>
> > > >>> gcc/testsuite/ChangeLog:
> > > >>>
> > > >>> * gcc.c-torture/compile/pr109919-1.c: New test.
> > > >> Thanks. I'll respin the targets that failed. If you don't hear from
> > > >> me, assume everything is happy again after this fix.
> > > >
> > > > Oh, I am going to test on aarch64-linux-gnu too just in case.
> > > > Expand is definitely something which I am not used to working on so I
> > > > figured I had made a mistake somewhere. I suspect I still made a
> > > > similar mistake later on too.
> > > I'm seeing some execution failures. Building H8 bits now to debug as
> > > it's the target I'm most familiar with. More info as it's available.
> >
> > Is H8 big-endian? I could have messed that up.
>
> If so, try the attached patch. I thought extract_bit_field's bitnum
> field was like a shift and not like how BIT_FIELD_REF is defined.
Forgot about poly modes requiring to use as_a <scalar_int_mode> to get
an integer mode's integer bitsize now.
So try this version (which now compiles).
Thanks,
Andrew
>
> Thanks,
> Andrew
>
> >
> > Thanks,
> > Andrew Pinski
> >
> > >
> > > jeff
diff --git a/gcc/expr.cc b/gcc/expr.cc
index 02f24c00148..050efcd0b00 100644
--- a/gcc/expr.cc
+++ b/gcc/expr.cc
@@ -12958,7 +12958,14 @@ expand_single_bit_test (location_t loc, enum tree_code code,
rtx inner0 = expand_expr (inner, NULL_RTX, VOIDmode, EXPAND_NORMAL);
- inner0 = extract_bit_field (inner0, 1, bitnum, 1, target,
+ int bitpos = bitnum;
+
+ scalar_int_mode imode = as_a <scalar_int_mode>(GET_MODE (inner0));
+
+ if (BYTES_BIG_ENDIAN)
+ bitpos = GET_MODE_BITSIZE (imode) - 1 - bitpos;
+
+ inner0 = extract_bit_field (inner0, 1, bitpos, 1, target,
operand_mode, mode, 0, NULL);
if (code == EQ_EXPR)
On 5/20/23 21:28, Andrew Pinski wrote:
> On Sat, May 20, 2023 at 8:25 PM Jeff Law <jeffreyalaw@gmail.com> wrote:
>>
>>
>>
>> On 5/20/23 21:05, Andrew Pinski wrote:
>>> On Sat, May 20, 2023 at 6:26 PM Jeff Law via Gcc-patches
>>> <gcc-patches@gcc.gnu.org> wrote:
>>>>
>>>>
>>>>
>>>> On 5/20/23 19:09, Andrew Pinski via Gcc-patches wrote:
>>>>> The problem is I used expand_expr with the target but
>>>>> we don't want to use the target here as it is the wrong
>>>>> mode for the original expression. The testcase would ICE
>>>>> deap down while trying to do a move to use the target.
>>>>> Anyways just calling expand_expr with NULL_EXPR fixes
>>>>> the issue.
>>>>>
>>>>> Committed as obvious after a bootstrap/test on x86_64-linux-gnu.
>>>>>
>>>>> PR middle-end/109919
>>>>>
>>>>> gcc/ChangeLog:
>>>>>
>>>>> * expr.cc (expand_single_bit_test): Don't use the
>>>>> target for expand_expr.
>>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>> * gcc.c-torture/compile/pr109919-1.c: New test.
>>>> Thanks. I'll respin the targets that failed. If you don't hear from
>>>> me, assume everything is happy again after this fix.
>>>
>>> Oh, I am going to test on aarch64-linux-gnu too just in case.
>>> Expand is definitely something which I am not used to working on so I
>>> figured I had made a mistake somewhere. I suspect I still made a
>>> similar mistake later on too.
>> I'm seeing some execution failures. Building H8 bits now to debug as
>> it's the target I'm most familiar with. More info as it's available.
>
> Is H8 big-endian? I could have messed that up.
It is. In fact, that does seem to be common across the failing targets.
I'll respin them again with your latest fix.
jeff
@@ -12956,7 +12956,7 @@ expand_single_bit_test (location_t loc, enum tree_code code,
intermediate_type = ops_unsigned ? unsigned_type : signed_type;
inner = fold_convert_loc (loc, intermediate_type, inner);
- rtx inner0 = expand_expr (inner, target, VOIDmode, EXPAND_NORMAL);
+ rtx inner0 = expand_expr (inner, NULL_RTX, VOIDmode, EXPAND_NORMAL);
inner0 = extract_bit_field (inner0, 1, bitnum, 1, target,
operand_mode, mode, 0, NULL);
new file mode 100644
@@ -0,0 +1,9 @@
+/* { dg-options "-fno-tree-dce -fno-tree-vrp" } */
+int a;
+int main() {
+ int b = 1;
+ while (a) {
+ short c = b && ((a || a) & (a * c));
+ }
+ return 0;
+}