[Bug middle-end/105874] New: [13 Regression] Incorrect codegen and ICE since g:ed6fd2aed58f2cca99f15331bf68999c0e6df370

[Bug middle-end/105874] New: [13 Regression] Incorrect codegen and ICE since g:ed6fd2aed58f2cca99f15331bf68999c0e6df370

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105874

            Bug ID: 105874
           Summary: [13 Regression] Incorrect codegen and ICE since
                    g:ed6fd2aed58f2cca99f15331bf68999c0e6df370
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code, wrong-code
          Severity: normal
          Priority: P3
         Component: middle-end
          Assignee: unassigned at gcc dot gnu.org
          Reporter: tnfchris at gcc dot gnu.org
                CC: sayle at gcc dot gnu.org
  Target Milestone: ---
            Target: aarch64*

In SPECCPU 2017 Leela no longer terminates since
g:ed6fd2aed58f2cca99f15331bf68999c0e6df370

Looking at the differences in the code, there seems to be a lot of additional
useless calculation around functions such as _ZN9FastBoard6is_eyeEii

Filtering through them it looks like the change is causing loads from
uninitialize d stack space.

Before the change the code generated

```
_ZN9FastBoard6is_eyeEii:
adrp    x3, <<diffable>>
mov     x4, #0x1ba4                     // #7076
add     x4, x0, x4
add     x3, x3, #0xb20
ldrh    w4, [x4, w2, sxtw #1]
ldr     w3, [x3, w1, sxtw #2]
tst     w4, w3
```

So it loaded from a fixed anchor into rdata.  After the change

```
_ZN9FastBoard6is_eyeEii:
sub     sp, sp, #0x20
mov     x4, #0x1ba4
add     x5, x0, x4
add     x4, sp, #0x8
ldrh    w5, [x5, w2, sxtw #1]
ldr     w4, [x4, w1, sxtw #2]
tst     w5, w4
```

So it allocated 32 bytes of stack and then decides to load from uninitialized
space at sp+0x8.

I tried to create a minimal reproducer but the compiler ICEs as you get close.
e.g. even the example from the ticket PR95126

struct small{ short a,b; signed char c; };
extern int func(struct small X);
void call_func(void)
{
    static struct small const s = { 1, 2, 0 };
    func(s);
}

ICEs at -O2 with:

during RTL pass: expand
../borked.c: In function 'call_func':
../borked.c:6:5: internal compiler error: in emit_move_insn, at expr.cc:4011
    6 |     func(s);
      |     ^~~~~~~
0x909253 emit_move_insn(rtx_def*, rtx_def*)
        /ci/work/5c94c4ced6ebfcd0/gcc/expr.cc:4011
0x7eda3f load_register_parameters
        /ci/work/5c94c4ced6ebfcd0/gcc/calls.cc:2192
0x7f2183 expand_call(tree_node*, rtx_def*, int)
        /ci/work/5c94c4ced6ebfcd0/gcc/calls.cc:3593
0x905ccb expand_expr_real_1(tree_node*, rtx_def*, machine_mode,
expand_modifier, rtx_def**, bool)
        /ci/work/5c94c4ced6ebfcd0/gcc/expr.cc:11621
0x8057e3 expand_expr
        /ci/work/5c94c4ced6ebfcd0/gcc/expr.h:301
0x8057e3 expand_call_stmt
        /ci/work/5c94c4ced6ebfcd0/gcc/cfgexpand.cc:2831
0x8057e3 expand_gimple_stmt_1
        /ci/work/5c94c4ced6ebfcd0/gcc/cfgexpand.cc:3869
0x8057e3 expand_gimple_stmt
        /ci/work/5c94c4ced6ebfcd0/gcc/cfgexpand.cc:4033
0x80a44b expand_gimple_tailcall
        /ci/work/5c94c4ced6ebfcd0/gcc/cfgexpand.cc:4079
0x80a44b expand_gimple_basic_block
        /ci/work/5c94c4ced6ebfcd0/gcc/cfgexpand.cc:6059
0x80cbbf execute
        /ci/work/5c94c4ced6ebfcd0/gcc/cfgexpand.cc:6811

So I can't really reduce it at this point.

[Bug middle-end/105874] [13 Regression] Incorrect codegen and ICE since g:ed6fd2aed58f2cca99f15331bf68999c0e6df370

[roger at nextmovesoftware dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105874

Roger Sayle <roger at nextmovesoftware dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |roger at nextmovesoftware dot com

--- Comment #1 from Roger Sayle <roger at nextmovesoftware dot com> ---
Hi Tamar.
I'm truly sorry for the inconvenience.  Can you try reducing again now that the
load_register_parameters issue with the "small const structs as immediate
constants" patch has been resolved?  A small failing testcase from Leela's
FastBoard::is_eye would be extremely helpful.  Thanks in advance.

[Bug middle-end/105874] [13 Regression] Incorrect codegen and ICE since g:ed6fd2aed58f2cca99f15331bf68999c0e6df370

[roger at nextmovesoftware dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105874

Roger Sayle <roger at nextmovesoftware dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2022-06-07
     Ever confirmed|0                           |1
           Assignee|unassigned at gcc dot gnu.org      |roger at nextmovesoftware dot com
   Target Milestone|---                         |13.0

--- Comment #2 from Roger Sayle <roger at nextmovesoftware dot com> ---
I believe I have a fix (that should probably also cure the ada bootstrap
failure).
I'm currently bootstraping and regression testing [passing EXPAND_MEMORY on
line 11196 of expr.cc].

[Bug middle-end/105874] [13 Regression] Incorrect codegen and ICE since g:ed6fd2aed58f2cca99f15331bf68999c0e6df370

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105874

--- Comment #3 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
(In reply to Roger Sayle from comment #1)
> Hi Tamar.
> I'm truly sorry for the inconvenience.  Can you try reducing again now that
> the load_register_parameters issue with the "small const structs as
> immediate constants" patch has been resolved?  A small failing testcase from
> Leela's FastBoard::is_eye would be extremely helpful.  Thanks in advance.

No Worries, I can see indeed the ICE is resolved, I'll minimize the benchmark.

Cheers!

[Bug middle-end/105874] [13 Regression] Incorrect codegen and ICE since g:ed6fd2aed58f2cca99f15331bf68999c0e6df370

[roger at nextmovesoftware dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105874

Roger Sayle <roger at nextmovesoftware dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #4 from Roger Sayle <roger at nextmovesoftware dot com> ---
Patch proposed.
https://gcc.gnu.org/pipermail/gcc-patches/2022-June/596391.html

[Bug middle-end/105874] [13 Regression] Incorrect codegen and ICE since g:ed6fd2aed58f2cca99f15331bf68999c0e6df370

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105874

--- Comment #5 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
Ah, great, Thanks Roger!

I did end up reducing it to:

template <int a> class b {
public:
  int c[a];
  int operator[](long d) const { return c[d]; }
};
class board {
  bool is_eye(int, int);
  static const b<2> e;
};
const b<2> board::e{{}};
bool board::is_eye(int d, int) {
  int f(e[d]);
  if (f)
    return false;
  return true;
}

but looks like you already have a patch :)

Nice patch btw!

[Bug middle-end/105874] [13 Regression] Incorrect codegen and ICE since g:ed6fd2aed58f2cca99f15331bf68999c0e6df370

[ebotcazou at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105874

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |matthias291999 at gmail dot com

--- Comment #6 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
*** Bug 105872 has been marked as a duplicate of this bug. ***

[Bug middle-end/105874] [13 Regression] Incorrect codegen and ICE since g:ed6fd2aed58f2cca99f15331bf68999c0e6df370

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105874

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Roger Sayle <sayle@gcc.gnu.org>:

https://gcc.gnu.org/g:b6e1373bd34aebbb512a03ea9a4e3c7acd955382

commit r13-1016-gb6e1373bd34aebbb512a03ea9a4e3c7acd955382
Author: Roger Sayle <roger@nextmovesoftware.com>
Date:   Wed Jun 8 20:43:03 2022 +0100

    PR middle-end/105874: Use EXPAND_MEMORY to fix ada bootstrap.

    Many thanks to Tamar Christina for filing PR middle-end/105874 indicating
    that SPECcpu 2017's Leela is failing on x86_64 due to a miscompilation
    of FastBoard::is_eye.  This function is much smaller and easier to work
    with than my previous hunt for the cause of the Ada bootstrap failures
    due to miscompilation somewhere in GCC (or one of the 131 places that
    the problematic form of optimization triggers during an ada bootstrap).

    It turns out the source of the miscompilation introduced by my recent
    patch is the distinction (during RTL expansion) of l-values and r-values.
    According to the documentation above expand_modifier, EXPAND_MEMORY
    should be used for lvalues (when a memory is required), and EXPAND_NORMAL
    for rvalues when a constant is permissible.  In what I'd like to consider
    a latent bug, the recursive call to expand_expr_real on line 11188 of
    expr.cc, in the case handling ARRAY_REF, COMPONENT_REF, BIT_FIELD_REF
    and ARRARY_RANGE_REF was passing EXPAND_NORMAL when it really required
    (the semantics of) EXPAND_MEMORY.  All the time that VAR_DECLs were
    being returned as memory this was fine, but as soon as we're able to
    optimize sort arrays into immediate constants, bad things happen.

    In the test case from Leela, we notice that the array s_eyemask
    always has DImode constant value { 4, 64 }, which is useful as
    an rvalue, but not when we need to index it as an lvalue, as in
    s_eyemask[color].  This also explains why everything being accepted
    by immediate_const_ctor_p (during an ada bootstrap) looks reasonable,
    what's incorrect is that we don't know how these structs/arrays are
    to be used.

    The fix is to ensure that we call expand_expr with EXPAND_MEMORY
    when processing the VAR_DECL's returned by get_inner_reference.

    2022-06-08  Roger Sayle  <roger@nextmovesoftware.com>

    gcc/ChangeLog
            PR middle-end/105874
            * expr.cc (expand_expr_real_1) <normal_inner_ref>:  New local
            variable tem_modifier for calculating the expand_modifier enum to
            use for expanding tem.  If tem is a VAR_DECL, use EXPAND_MEMORY.

    gcc/testsuite/ChangeLog
            PR middle-end/105874
            * g++.dg/opt/pr105874.C: New test case.

[Bug middle-end/105874] [13 Regression] Incorrect codegen and ICE since g:ed6fd2aed58f2cca99f15331bf68999c0e6df370

[tnfchris at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105874

--- Comment #8 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
Can confirm that the benchmark works again.

Thanks!

[Bug middle-end/105874] [13 Regression] Incorrect codegen and ICE since g:ed6fd2aed58f2cca99f15331bf68999c0e6df370

[roger at nextmovesoftware dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105874

Roger Sayle <roger at nextmovesoftware dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #9 from Roger Sayle <roger at nextmovesoftware dot com> ---
Thanks again Tamar for confirming this bug is now fixed (alas I don't have an
aarch64 to check for myself).  Sorry again for the inconvenience.

[Bug middle-end/105874] [13 Regression] Incorrect codegen and ICE since g:ed6fd2aed58f2cca99f15331bf68999c0e6df370

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105874

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Eric Botcazou <ebotcazou@gcc.gnu.org>:

https://gcc.gnu.org/g:90129d39ca0fc1d2ac9cf960379feccea878bd90

commit r13-1378-g90129d39ca0fc1d2ac9cf960379feccea878bd90
Author: Eric Botcazou <ebotcazou@adacore.com>
Date:   Fri Jul 1 09:59:05 2022 +0200

    Amend fix for PR middle-end/105874

    The original fix is very likely too big a hammer.

    gcc/
            PR middle-end/105874
            * expr.cc (expand_expr_real_1) <normal_inner_ref>: Force
            EXPAND_MEMORY for the expansion of the inner reference only
            in the usual cases where a memory reference is required.