[Bug tree-optimization/97421] New: [10/11 Regression] aarch64: Wrong code with -O2 -fmodulo-sched since r10-1318-ga7e8a46

[Bug tree-optimization/97421] New: [10/11 Regression] aarch64: Wrong code with -O2 -fmodulo-sched since r10-1318-ga7e8a46

[acoplan at gcc dot gnu.org]

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

            Bug ID: 97421
           Summary: [10/11 Regression] aarch64: Wrong code with -O2
                    -fmodulo-sched since r10-1318-ga7e8a46
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: acoplan at gcc dot gnu.org
  Target Milestone: ---

AArch64 GCC miscompiles the following testcase:

int a, b, d, e;
int *volatile c = &a;

__attribute__((noinline))
void f(void)
{
  for (int g = 2; g >= 0; g--) {
    d = 0;
    for (b = 0; b <= 2; b++)
      ;
    e = *c;
  }
}

int main(void)
{
  f();
  if (b != 3)
    __builtin_abort();
}

with -O2 -fmodulo-sched since
r10-1318-ga7e8a463cd1dbaccf6e7c4fa888768fcd257a30f. Removing -fmodulo-sched,
the testcase is compiled correctly (tested at -O0 through -O3).

Here is the generated code (current trunk) for f at -O2:

f:
        mov     w1, 3
        adrp    x0, .LANCHOR0
        adrp    x3, .LANCHOR1
        mov     w4, w1
        add     x0, x0, :lo12:.LANCHOR0 // x0 <- &d
        add     x3, x3, :lo12:.LANCHOR1 // x3 <- &c
.L2:
        ldr     x2, [x3]      // x2 <- c
        stp     wzr, w4, [x0] // d = 0, b = 3
        subs    w1, w1, #1
        ldr     w2, [x2]      // w2 <- *c
        str     w2, [x0, 8]   // e <- w2
        bne     .L2
        ret

and the code for f with -O2 -fmodulo-sched:

f:
        mov     w1, 2
        adrp    x0, .LANCHOR0
        mov     w4, w1
        add     x0, x0, :lo12:.LANCHOR0
        adrp    x3, .LANCHOR1
        add     x3, x3, :lo12:.LANCHOR1
        ldr     x2, [x3]    // x2 <- c
        str     w1, [x0, 4] // b = 2
.L2:
        str     wzr, [x0]   // d = 0
        ldr     w2, [x2]    // w2 <- *c
        subs    w1, w1, #1  
        str     w2, [x0, 8] // e <- w2
        ldr     x2, [x3]    // x2 <- c
        str     w4, [x0, 4] // b = 2
        bne     .L2
        ldr     w2, [x2]    // w2 <- *c
        str     w2, [x0, 8] // e <- w2
        ret

In the latter case we end up with b = 2 and make the call to __builtin_abort().

[Bug tree-optimization/97421] [10/11 Regression] aarch64: Wrong code with -O2 -fmodulo-sched since r10-1318-ga7e8a46

[acoplan at gcc dot gnu.org]

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

Alex Coplan <acoplan at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |aarch64
      Known to fail|                            |11.0
   Target Milestone|---                         |10.3

[Bug rtl-optimization/97421] [10/11 Regression] aarch64: Wrong code with -O2 -fmodulo-sched since r10-1318-ga7e8a46

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|tree-optimization           |rtl-optimization

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
That was clearly a correctness fix doing less TBAA.  But it eventually triggers
a latent bug in modulo-sched or sched-deps.

[Bug rtl-optimization/97421] [10/11 Regression] aarch64: Wrong code with -O2 -fmodulo-sched since r10-1318-ga7e8a46

[rguenth at gcc dot gnu.org]

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
So maybe try bisecting/reproducing with -fno-strict-aliasing?

[Bug rtl-optimization/97421] [10/11 Regression] aarch64: Wrong code with -O2 -fmodulo-sched since r10-1318-ga7e8a46

[acoplan at gcc dot gnu.org]

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

--- Comment #3 from Alex Coplan <acoplan at gcc dot gnu.org> ---
> So maybe try bisecting/reproducing with -fno-strict-aliasing?

Ah, yes, I can reproduce before that revision with -fno-strict-aliasing. I'll
re-bisect, thanks.

[Bug rtl-optimization/97421] aarch64: Wrong code with -O2 -fmodulo-sched

[acoplan at gcc dot gnu.org]

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

Alex Coplan <acoplan at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[10/11 Regression] aarch64: |aarch64: Wrong code with
                   |Wrong code with -O2         |-O2 -fmodulo-sched
                   |-fmodulo-sched since        |
                   |r10-1318-ga7e8a46           |

--- Comment #4 from Alex Coplan <acoplan at gcc dot gnu.org> ---
With -fno-strict-aliasing I can reproduce this back to
r6-3620-gcdc1afa3c04e6ddf2d2119a4144754cabcfe419a which introduces Cortex-A53
scheduling.

Not sure if it's possible to reproduce before that revision.

[Bug rtl-optimization/97421] aarch64: Wrong code with -O2 -fmodulo-sched

[zhroma at gcc dot gnu.org]

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

Roman Zhuykov <zhroma at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |zhroma at gcc dot gnu.org

--- Comment #5 from Roman Zhuykov <zhroma at gcc dot gnu.org> ---
Created attachment 49604
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49604&action=edit
Proposed patch

[Bug rtl-optimization/97421] aarch64: Wrong code with -O2 -fmodulo-sched

[zhroma at gcc dot gnu.org]

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

Roman Zhuykov <zhroma at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2020-11-20
                 CC|                            |abel at gcc dot gnu.org,
                   |                            |amonakov at gcc dot gnu.org,
                   |                            |rguenth at gcc dot gnu.org
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |NEW
           Assignee|unassigned at gcc dot gnu.org      |zhroma at gcc dot gnu.org

--- Comment #6 from Roman Zhuykov <zhroma at gcc dot gnu.org> ---
Thanks for catching this!

When modulo sched succeded and found that counter register is const-initialized
in prologue it wrongly directly adjust the initization without checking that
register may be read a bit later in prologue.

I've attached the patch I'm testing.

[Bug rtl-optimization/97421] aarch64: Wrong code with -O2 -fmodulo-sched

[zhroma at gcc dot gnu.org]

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

--- Comment #7 from Roman Zhuykov <zhroma at gcc dot gnu.org> ---
*** Bug 97851 has been marked as a duplicate of this bug. ***

[Bug rtl-optimization/97421] aarch64: Wrong code with -O2 -fmodulo-sched

[zhroma at gcc dot gnu.org]

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

--- Comment #8 from Roman Zhuykov <zhroma at gcc dot gnu.org> ---
*** Bug 97440 has been marked as a duplicate of this bug. ***

[Bug rtl-optimization/97421] aarch64: Wrong code with -O2 -fmodulo-sched

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Roman Zhuykov <zhroma@gcc.gnu.org>:

https://gcc.gnu.org/g:4eb8f93d026eaa1de9b4820337069f3ce3465cd0

commit r11-5760-g4eb8f93d026eaa1de9b4820337069f3ce3465cd0
Author: Roman Zhuykov <zhroma@ispras.ru>
Date:   Sat Dec 5 18:45:27 2020 +0300

    modulo-sched: Carefully process loop counter initialization [PR97421]

    Do not allow direct adjustment of pre-header initialization instruction for
    count register if is read in some instruction below in that basic block.

    gcc/ChangeLog:

            PR rtl-optimization/97421
            * modulo-sched.c (generate_prolog_epilog): Remove forward
            declaration, adjust last argument name and type.
            (const_iteration_count): Add bool pointer parameter to return
            whether count register is read in pre-header after its
            initialization.
            (sms_schedule): Fix count register initialization adjustment
            procedure according to what const_iteration_count said.

    gcc/testsuite/ChangeLog:

            PR rtl-optimization/97421
            * gcc.c-torture/execute/pr97421-1.c: New test.
            * gcc.c-torture/execute/pr97421-2.c: New test.
            * gcc.c-torture/execute/pr97421-3.c: New test.

[Bug rtl-optimization/97421] aarch64: Wrong code with -O2 -fmodulo-sched

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Roman Zhuykov
<zhroma@gcc.gnu.org>:

https://gcc.gnu.org/g:2850df4edce86b13d503e5bf636a874fb21198fc

commit r8-10694-g2850df4edce86b13d503e5bf636a874fb21198fc
Author: Roman Zhuykov <zhroma@ispras.ru>
Date:   Thu Dec 24 19:40:07 2020 +0300

    modulo-sched: Carefully process loop counter initialization [PR97421]

    Do not allow direct adjustment of pre-header initialization instruction for
    count register if is read in some instruction below in that basic block.

    gcc/ChangeLog:

            PR rtl-optimization/97421
            * modulo-sched.c (generate_prolog_epilog): Remove forward
            declaration, adjust last argument name and type.
            (const_iteration_count): Add bool pointer parameter to return
            whether count register is read in pre-header after its
            initialization.
            (sms_schedule): Fix count register initialization adjustment
            procedure according to what const_iteration_count said.

    gcc/testsuite/ChangeLog:

            PR rtl-optimization/97421
            * gcc.c-torture/execute/pr97421-1.c: New test.
            * gcc.c-torture/execute/pr97421-2.c: New test.
            * gcc.c-torture/execute/pr97421-3.c: New test.

    (cherry picked from commit 4eb8f93d026eaa1de9b4820337069f3ce3465cd0)

[Bug rtl-optimization/97421] aarch64: Wrong code with -O2 -fmodulo-sched

[cvs-commit at gcc dot gnu.org]

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

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Roman Zhuykov
<zhroma@gcc.gnu.org>:

https://gcc.gnu.org/g:2435878321b053aebb2e2a886465662199542875

commit r9-9129-g2435878321b053aebb2e2a886465662199542875
Author: Roman Zhuykov <zhroma@ispras.ru>
Date:   Thu Dec 24 19:42:14 2020 +0300

    modulo-sched: Carefully process loop counter initialization [PR97421]

    Do not allow direct adjustment of pre-header initialization instruction for
    count register if is read in some instruction below in that basic block.

    gcc/ChangeLog:

            PR rtl-optimization/97421
            * modulo-sched.c (generate_prolog_epilog): Remove forward
            declaration, adjust last argument name and type.
            (const_iteration_count): Add bool pointer parameter to return
            whether count register is read in pre-header after its
            initialization.
            (sms_schedule): Fix count register initialization adjustment
            procedure according to what const_iteration_count said.

    gcc/testsuite/ChangeLog:

            PR rtl-optimization/97421
            * gcc.c-torture/execute/pr97421-1.c: New test.
            * gcc.c-torture/execute/pr97421-2.c: New test.
            * gcc.c-torture/execute/pr97421-3.c: New test.

    (cherry picked from commit 4eb8f93d026eaa1de9b4820337069f3ce3465cd0)

[Bug rtl-optimization/97421] aarch64: Wrong code with -O2 -fmodulo-sched

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Roman Zhuykov
<zhroma@gcc.gnu.org>:

https://gcc.gnu.org/g:0963fa93dcc47d290d1597ff88ff162f5bdd1b13

commit r10-9171-g0963fa93dcc47d290d1597ff88ff162f5bdd1b13
Author: Roman Zhuykov <zhroma@ispras.ru>
Date:   Thu Dec 24 19:43:40 2020 +0300

    modulo-sched: Carefully process loop counter initialization [PR97421]

    Do not allow direct adjustment of pre-header initialization instruction for
    count register if is read in some instruction below in that basic block.

    gcc/ChangeLog:

            PR rtl-optimization/97421
            * modulo-sched.c (generate_prolog_epilog): Remove forward
            declaration, adjust last argument name and type.
            (const_iteration_count): Add bool pointer parameter to return
            whether count register is read in pre-header after its
            initialization.
            (sms_schedule): Fix count register initialization adjustment
            procedure according to what const_iteration_count said.

    gcc/testsuite/ChangeLog:

            PR rtl-optimization/97421
            * gcc.c-torture/execute/pr97421-1.c: New test.
            * gcc.c-torture/execute/pr97421-2.c: New test.
            * gcc.c-torture/execute/pr97421-3.c: New test.

    (cherry picked from commit 4eb8f93d026eaa1de9b4820337069f3ce3465cd0)

[Bug rtl-optimization/97421] aarch64: Wrong code with -O2 -fmodulo-sched

[zhroma at gcc dot gnu.org]

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

Roman Zhuykov <zhroma at gcc dot gnu.org> changed:

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

--- Comment #13 from Roman Zhuykov <zhroma at gcc dot gnu.org> ---
Fixed on all active branches.