[Bug tree-optimization/100453] New: wrong code at -O1 and above

[Bug tree-optimization/100453] New: wrong code at -O1 and above

[zhendong.su at inf dot ethz.ch]

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

            Bug ID: 100453
           Summary: wrong code at -O1 and above
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: zhendong.su at inf dot ethz.ch
  Target Milestone: ---

It seems to be a recent regression.

[513] % gcctk -v
Using built-in specs.
COLLECT_GCC=gcctk
COLLECT_LTO_WRAPPER=/local/suz-local/software/local/gcc-trunk/libexec/gcc/x86_64-pc-linux-gnu/12.0.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: ../gcc-trunk/configure --disable-bootstrap
--prefix=/local/suz-local/software/local/gcc-trunk --enable-languages=c,c++
--disable-werror --enable-multilib --with-system-zlib
Thread model: posix
Supported LTO compression algorithms: zlib
gcc version 12.0.0 20210506 (experimental) [master revision
1698f496c5e:27b373b33d4:a1ac9ffb5a7f44b2e2633b7265c21ce803c8e854] (GCC) 
[514] % 
[514] % gcctk -O0 small.c; ./a.out
[515] % gcc110 -O1 small.c; ./a.out
[516] % 
[516] % gcctk -O1 small.c
[517] % ./a.out
Segmentation fault
[518] % 
[518] % cat small.c
struct a {
  int b : 4;
} d;
static int c, e;
static const struct a f;
static void g(const struct a h) {
  for (; c < 1; c++)
    d = h;
  e = h.b;
  c = h.b;
}
int main() {
  g(f);
  return 0;
}

[Bug tree-optimization/100453] [12 Regression] wrong code at -O1 and above since r12-434

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|unknown                     |12.0
   Last reconfirmed|                            |2021-05-06
   Target Milestone|---                         |12.0
     Ever confirmed|0                           |1
            Summary|wrong code at -O1 and above |[12 Regression] wrong code
                   |                            |at -O1 and above since
                   |                            |r12-434
             Status|UNCONFIRMED                 |NEW
                 CC|                            |ebotcazou at gcc dot gnu.org,
                   |                            |jakub at gcc dot gnu.org

--- Comment #1 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r12-434-g93f8cb4965cebee125f96376367f05e18ee5749b

[Bug tree-optimization/100453] [12 Regression] wrong code at -O1 and above since r12-434

[ebotcazou at gcc dot gnu.org]

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

--- Comment #2 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
It's SRA writing into a TREE_READONLY parameter:

void g (const struct a h)
{
  const <unnamed-signed:4> h$b;
  int c.0_1;
  int _2;
  int c.1_3;
  <unnamed-signed:4> _4;
  int _5;
  <unnamed-signed:4> _6;
  int _7;

  <bb 2> :
  h$b_14 = MEM[(struct a *)&h].b;
  goto <bb 4>; [INV]

  <bb 3> :
  MEM[(struct a *)&h].b = h$b_14;

[Bug tree-optimization/100453] [12 Regression] wrong code at -O1 and above since r12-434

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jamborm at gcc dot gnu.org

--- Comment #3 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Mine.

[Bug tree-optimization/100453] [12 Regression] wrong code at -O1 and above since r12-434

[jamborm at gcc dot gnu.org]

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

--- Comment #4 from Martin Jambor <jamborm at gcc dot gnu.org> ---
I proposed a patch to address this on the mailing list:
https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570267.html

[Bug tree-optimization/100453] [12 Regression] wrong code at -O1 and above since r12-434

[ebotcazou at gcc dot gnu.org]

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

--- Comment #5 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
> I proposed a patch to address this on the mailing list:
> https://gcc.gnu.org/pipermail/gcc-patches/2021-May/570267.html

Thanks!

[Bug tree-optimization/100453] [12 Regression] wrong code at -O1 and above since r12-434

[cvs-commit at gcc dot gnu.org]

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

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>:

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

commit r12-782-gca9bb74a5f856ccdceb4797f18b0a4ac8f49d069
Author: Martin Jambor <mjambor@suse.cz>
Date:   Thu May 13 23:26:32 2021 +0200

    tree-sra: Avoid refreshing into const base decls (PR 100453)

    When SRA transforms an assignment where the RHS is an aggregate decl
    that it creates replacements for, the (least efficient) fallback
    method of dealing with them is to store all the replacements back into
    the original decl and then let the original assignment takes itc
    sourse.

    That of course should not need to be done for TREE_READONLY bases
    which cannot change contents.  The SRA code handled this situation in
    one of two necessary places but only for DECL_IN_CONSTANT_POOL const
    decls, this patch modifies both to check TREE_READONLY.

    gcc/ChangeLog:

    2021-05-12  Martin Jambor  <mjambor@suse.cz>

            PR tree-optimization/100453
            * tree-sra.c (sra_modify_assign): All const base accesses do not
            need refreshing, not just those from decl_pool.
            (sra_modify_assign): Do not refresh into a const base decl.

    gcc/testsuite/ChangeLog:

    2021-05-12  Martin Jambor  <mjambor@suse.cz>

            PR tree-optimization/100453
            * gcc.dg/tree-ssa/pr100453.c: New test.

[Bug tree-optimization/100453] [12 Regression] wrong code at -O1 and above since r12-434

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

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

--- Comment #7 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Fixed on trunk.  Miscompilation has only been observed there so hopefully that
is enough.

[Bug tree-optimization/100453] [12 Regression] wrong code at -O1 and above since r12-434

[ebotcazou at gcc dot gnu.org]

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

--- Comment #8 from Eric Botcazou <ebotcazou at gcc dot gnu.org> ---
Martin, did you test Ada with your patch?  It appears that it either causes
bootstrap to fail (PR bootstrap/100597) or miscompiles the Ada compiler:

                === acats tests ===
FAIL:   c41325a
FAIL:   c45347d
FAIL:   c74402a
FAIL:   c95085m
FAIL:   cc3601a

                === acats Summary ===
# of expected passes            2323
# of unexpected failures        5
Native configuration is x86_64-suse-linux-gnu

                === gnat tests ===


Running target unix
FAIL: gnat.dg/addr12.adb (test for excess errors)
UNRESOLVED: gnat.dg/addr12.adb compilation failed to produce executable
FAIL: gnat.dg/addr12_a.adb (test for excess errors)
FAIL: gnat.dg/bip_overlay.adb (test for excess errors)
FAIL: gnat.dg/global.adb (test for excess errors)
FAIL: gnat.dg/spark1.adb  (test for errors, line 8)
FAIL: gnat.dg/spark1.adb (test for excess errors)
FAIL: gnat.dg/sync2.adb (test for excess errors)
FAIL: gnat.dg/synchronized1.adb (test for excess errors)

                === gnat Summary ===

# of expected passes            3360
# of unexpected failures        8
# of expected failures          23
# of unresolved testcases       1
# of unsupported tests          3


Most parameters are read-only in Ada so it's quite a good testbed...

[Bug tree-optimization/100453] [12 Regression] wrong code at -O1 and above since r12-434

[cvs-commit at gcc dot gnu.org]

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

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

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

commit r12-809-gaf42043e6618e69187b47f37dac870763c01e20f
Author: Martin Jambor <mjambor@suse.cz>
Date:   Sat May 15 10:11:12 2021 +0200

    Revert "tree-sra: Avoid refreshing into const base decls (PR 100453)"

    This reverts commit ca9bb74a5f856ccdceb4797f18b0a4ac8f49d069.
    ...because of Ada issues I did not catch with original testing.

    gcc/ChangeLog:

    2021-05-12  Martin Jambor  <mjambor@suse.cz>

            Revert:
            PR tree-optimization/100453
            * tree-sra.c (sra_modify_assign): All const base accesses do not
            need refreshing, not just those from decl_pool.
            (sra_modify_assign): Do not refresh into a const base decl.

    gcc/testsuite/ChangeLog:

    2021-05-12  Martin Jambor  <mjambor@suse.cz>

            Revert:
            PR tree-optimization/100453
            * gcc.dg/tree-ssa/pr100453.c: New test.

[Bug tree-optimization/100453] [12 Regression] wrong code at -O1 and above since r12-434

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

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

--- Comment #10 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Only now I realized I do not test Ada on aarch64 (which I used to test the
const_decl_pool stuff).  I have reverted the patch to fix bootstrap and will
have another look at this next week. Sorry for the breakage.

[Bug tree-optimization/100453] [12 Regression] wrong code at -O1 and above since r12-434

[ebotcazou at gcc dot gnu.org]

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

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

[Bug tree-optimization/100453] [12 Regression] wrong code at -O1 and above since r12-434

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |suochenyao at 163 dot com

--- Comment #12 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 100994 has been marked as a duplicate of this bug. ***

[Bug tree-optimization/100453] [12 Regression] wrong code at -O1 and above since r12-434

[jamborm at gcc dot gnu.org]

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

--- Comment #13 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Another attempt to fix this:
https://gcc.gnu.org/pipermail/gcc-patches/2021-June/572814.html

[Bug tree-optimization/100453] [12 Regression] wrong code at -O1 and above since r12-434

[pinskia at gcc dot gnu.org]

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

--- Comment #14 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
*** Bug 101080 has been marked as a duplicate of this bug. ***

[Bug tree-optimization/100453] [12 Regression] wrong code at -O1 and above since r12-434

[cvs-commit at gcc dot gnu.org]

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

--- Comment #15 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Martin Jambor <jamborm@gcc.gnu.org>:

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

commit r12-1529-gd7deee423f993bee8ee440f6fe0c9126c316c64b
Author: Martin Jambor <mjambor@suse.cz>
Date:   Wed Jun 16 13:18:46 2021 +0200

    tree-sra: Do not refresh readonly decls (PR 100453)

    When SRA transforms an assignment where the RHS is an aggregate decl
    that it creates replacements for, the (least efficient) fallback
    method of dealing with them is to store all the replacements back into
    the original decl and then let the original assignment takes its
    course.

    That of course should not need to be done for TREE_READONLY bases
    which cannot change contents.  The SRA code handled this situation
    only for DECL_IN_CONSTANT_POOL const decls, this patch modifies the
    check so that it tests for TREE_READONLY and I also looked at all
    other callers of generate_subtree_copies and added checks to another
    one dealing with the same exact situation and one which deals with it
    in a non-assignment context.

    gcc/ChangeLog:

    2021-06-11  Martin Jambor  <mjambor@suse.cz>

            PR tree-optimization/100453
            * tree-sra.c (create_access): Disqualify any const candidates
            which are written to.
            (sra_modify_expr): Do not store sub-replacements back to a const
base.
            (handle_unscalarized_data_in_subtree): Likewise.
            (sra_modify_assign): Likewise.  Earlier, use TREE_READONLy test
            instead of constant_decl_p.

    gcc/testsuite/ChangeLog:

    2021-06-10  Martin Jambor  <mjambor@suse.cz>

            PR tree-optimization/100453
            * gcc.dg/tree-ssa/pr100453.c: New test.

[Bug tree-optimization/100453] [12 Regression] wrong code at -O1 and above since r12-434

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

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

--- Comment #16 from Martin Jambor <jamborm at gcc dot gnu.org> ---
Fixed.