[Bug c/111807] New: ice in verify_sra_access_forest with -O1

[Bug c/111807] New: ice in verify_sra_access_forest with -O1

[dcb314 at hotmail dot com]

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

            Bug ID: 111807
           Summary: ice in verify_sra_access_forest with -O1
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dcb314 at hotmail dot com
  Target Milestone: ---

Created attachment 56106
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56106&action=edit
gzipped C source code

The attached C code, when compiled by recent gcc, does this:

$ ~/gcc/results/bin/gcc -c -w -O1 bug965.c 
./buildData/in/in.20163.c: In function ‘func_57’:
./buildData/in/in.20163.c:1769:18: note: the ABI for passing parameters with
128-byte alignment has changed in GCC 4.6
during GIMPLE pass: esra
./buildData/in/in.20163.c: In function ‘func_40’:
./buildData/in/in.20163.c:2878:1: internal compiler error: in
verify_sra_access_forest, at tree-sra.cc:2414
0xfb289d verify_sra_access_forest(access*)
        /home/dcb38/gcc/working/gcc/../../trunk.year/gcc/tree-sra.cc:2414
0xfb2a2a verify_all_sra_access_forests()
        /home/dcb38/gcc/working/gcc/../../trunk.year/gcc/tree-sra.cc:2465
0xfb4672 analyze_all_variable_accesses()
        /home/dcb38/gcc/working/gcc/../../trunk.year/gcc/tree-sra.cc:3534

The bug first seems to appear sometime between git hash g:530babc2058be5f2
and g:78dd49f387c515a6, a range of 52 commits.

I will have a go at a reduction.

[Bug c/111807] ice in verify_sra_access_forest with -O1

[dcb314 at hotmail dot com]

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

--- Comment #1 from David Binderman <dcb314 at hotmail dot com> ---
Reduced C code seems to be:

struct S0 {
  unsigned f2 : 7
} func_40() {
  struct S0 l_4827[10];
  l_4827[5] = l_4827[9];
  0 || l_4827[9].f2;
}

[Bug c/111807] ice in verify_sra_access_forest with -O1

[dcb314 at hotmail dot com]

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

David Binderman <dcb314 at hotmail dot com> changed:

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

--- Comment #2 from David Binderman <dcb314 at hotmail dot com> ---
I've had a go at a git bisect and the current range is 
g:d53d20a940efe426 .. g:24eaada73def58a6, which is 14 commits.

Richard's two commits (g:6decda1a35be5764101987c210b5693a0d914e58,
g:35b5bb475375dba4ea9101d6db13a6012c4e84ca) look highly likely.

[Bug c/111807] ice in verify_sra_access_forest with -O1

[dcb314 at hotmail dot com]

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

--- Comment #3 from David Binderman <dcb314 at hotmail dot com> ---
Second test case:

int func_40_l_118;

struct S0 {
  signed f1 : 1;
};

void func_40() {
  struct S0 l_103[16];
  *l_103 = l_103[15];
  l_103[15].f1 &&func_40_l_118;
}

[Bug tree-optimization/111807] [14 Regression] ice in verify_sra_access_forest with -O1 and bitfields in some cases

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c                           |tree-optimization
   Target Milestone|---                         |14.0
            Summary|ice in                      |[14 Regression] ice in
                   |verify_sra_access_forest    |verify_sra_access_forest
                   |with -O1                    |with -O1 and bitfields in
                   |                            |some cases
           Keywords|                            |ice-on-valid-code

[Bug tree-optimization/111807] [14 Regression] ice in verify_sra_access_forest with -O1 and bitfields in some cases

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever confirmed|0                           |1
   Last reconfirmed|                            |2023-10-14

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed.

[Bug tree-optimization/111807] [14 Regression] ice in verify_sra_access_forest with -O1 and bitfields in some cases

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |zhendong.su at inf dot ethz.ch

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

[Bug tree-optimization/111807] [14 Regression] ice in verify_sra_access_forest with -O1 and bitfields in some cases

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|unassigned at gcc dot gnu.org      |rguenth at gcc dot gnu.org
            Version|unknown                     |14.0
             Status|NEW                         |ASSIGNED

--- Comment #6 from Richard Biener <rguenth at gcc dot gnu.org> ---
I will take a look.

[Bug tree-optimization/111807] [14 Regression] ice in verify_sra_access_forest with -O1 and bitfields in some cases

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
I think the issue is we are doing propagate_subaccesses_from_rhs with the
LHS l_4827[5] and the RHS access l_4827[9].f2.  We then end up with
build_ref_for_model calling build_reconstructed_reference but that will
simply re-construct the exact same reference with the same offset as the
model rather than the new desired offset ..

Possibly the mistake is setting ->grp_same_access_path here?

I wonder how this ever works for offset < model->offset and thus if the
fix is actually

-         && offset <= model->offset
+         && offset == model->offset

(gdb) p base
$59 = <var_decl 0x7ffff6d4ecf0 l_4827>
(gdb) p debug_generic_expr (model->expr)
BIT_FIELD_REF <l_4827[9], 8, 0>
(gdb) p model->offset
$61 = 288
(gdb) p offset
$62 = 160

that fixes the issue.

[Bug tree-optimization/111807] [14 Regression] ice in verify_sra_access_forest with -O1 and bitfields in some cases

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:

https://gcc.gnu.org/g:3aaf704bca3e01c72d9fe6de01ae5416dba6b5db

commit r14-4677-g3aaf704bca3e01c72d9fe6de01ae5416dba6b5db
Author: Richard Biener <rguenther@suse.de>
Date:   Mon Oct 16 12:54:28 2023 +0200

    tree-optimization/111807 - ICE in verify_sra_access_forest

    The following addresses build_reconstructed_reference failing to
    build references with a different offset than the models and thus
    the caller conditional being off.  This manifests when attempting
    to build a ref with offset 160 from the model BIT_FIELD_REF <l_4827[9], 8,
0>
    onto the same base l_4827 but the models offset being 288.  This
    cannot work for any kind of ref I can think of, not just with
    BIT_FIELD_REFs.

            PR tree-optimization/111807
            * tree-sra.cc (build_ref_for_model): Only call
            build_reconstructed_reference when the offsets are the same.

            * gcc.dg/torture/pr111807.c: New testcase.

[Bug tree-optimization/111807] [14 Regression] ice in verify_sra_access_forest with -O1 and bitfields in some cases

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed.

[Bug tree-optimization/111807] [14 Regression] ice in verify_sra_access_forest with -O1 and bitfields in some cases

[cvs-commit at gcc dot gnu.org]

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

--- Comment #10 from GCC 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:96e1978b213482fc4c25693b91ae2ead481af620

commit r14-6535-g96e1978b213482fc4c25693b91ae2ead481af620
Author: Martin Jambor <mjambor@suse.cz>
Date:   Thu Dec 14 11:09:06 2023 +0100

    SRA: Relax requirements to use build_reconstructed_reference (PR 111807)

    This patch half-reverts 3aaf704bca3e and replaces it with a fix with
    relaxed requiremets for invoking build_reconstructed_reference in
    build_ref_for_model.

    build_ref_for_model/build_ref_for_offset is used in two slightly
    different contexts. The first is when we are looking at an assignmernt
    like

       p->field_A.field_B = s.field_B;

    and we have a replacements for e.g. s.field_B.field_C.field_D and we
    want to store them directly to p->field_A.field_B.field_C.field_D (as
    opposed to going through s or using a MEM_REF based in
    p->field_A.field_B).  In this case, the offset of the
    "model" (s.field_B.field_C.field_D) within this can be different than
    offset within the LHS that we want to reach (field_C.field_D within
    the "base" p->field_A.field_B).  Patch 3aaf704bca3e has caused us to
    unnecessarily create MEM_REFs for these situations.  These uses of
    build_ref_for_model work with the relaxed condition just fine.

    The second, problematic, context is when somewhere in the function we
    have an assignment

      s.field_A = t.field_A.field_B;

    and we are creating an access structure to represent s.field_A.field_B
    even if it is not actually accessed in the original input.  This is
    done after scanning the entire function body and we need to construct
    a "universal" reference to s.field_A.field_B.  In this case the "base"
    is "s" and it has to be the DECL itself and not some reference for it
    because for arbitrary references we need a GSI pointing to a statement
    which we don't have, the reference is supposed to be universal.

    But then using build_ref_for_model and within it
    build_reconstructed_reference misbihaves if the expression contains
    any ARRAY_REFs.  In the first case those are fine because as we
    eventually reach the aggregate type that matches a real LHS or RHS, we
    know we we can just bolt the rest of the references onto it and end up
    with the correct overall reference.  However when dealing with

       s.array[1].field_A = s.array[2].field_B;

    we cannot just bolt array[2] reference when we want array[1] but that
    is exactly what happens when we use build_reconstructed_reference and
    keep it walking all the way to s.

    I was consiering making all users of the second kind use directly
    build_ref_for_offset instead of build_ref_for_model but the latter
    also handles COMPONENT_REFs to bit-fields which the former does not.
    THerefore I have deided to use the NULL-ness of GSI as an indicator
    how strict we need to be.  I have changed the function comment to
    reflect that.

    I have been able to observe diambiguation improvements with this patch
    over currenct master, we do successfuly manage a few more
    aliasing_component_refs_p disambiguations when compiling cc1, going
    from:

      Alias oracle query stats:
        refs_may_alias_p: 94354287 disambiguations, 106279231 queries
        ref_maybe_used_by_call_p: 1572511 disambiguations, 95618222 queries
        call_may_clobber_ref_p: 649273 disambiguations, 659371 queries
        stmt_kills_ref_p: 142342 kills, 8407309 queries
        nonoverlapping_component_refs_p: 19 disambiguations, 10227 queries
        nonoverlapping_refs_since_match_p: 15665 disambiguations, 52585 must
overlaps, 68893 queries
        aliasing_component_refs_p: 67090 disambiguations, 3081766 queries
        TBAA oracle: 22675296 disambiguations 61781978 queries
                     14045969 are in alias set 0
                     10997085 queries asked about the same object
                     153 queries asked about the same alias set
                     0 access volatile
                     12485774 are dependent in the DAG
                     1577701 are aritificially in conflict with void *

      Modref stats:
        modref kill: 832 kills, 19399 queries
        modref use: 50760 disambiguations, 1825109 queries
        modref clobber: 1371014 disambiguations, 40152535 queries
        5190238 tbaa queries (0.129263 per modref query)
        1341663 base compares (0.033414 per modref query)

      PTA query stats:
        pt_solution_includes: 36784427 disambiguations, 46141175 queries
        pt_solutions_intersect: 4519387 disambiguations, 17081996 queries

    to:

      Alias oracle query stats:
        refs_may_alias_p: 94354083 disambiguations, 106278948 queries
        ref_maybe_used_by_call_p: 1572511 disambiguations, 95618018 queries
        call_may_clobber_ref_p: 649273 disambiguations, 659371 queries
        stmt_kills_ref_p: 142342 kills, 8407310 queries
        nonoverlapping_component_refs_p: 19 disambiguations, 10227 queries
        nonoverlapping_refs_since_match_p: 15665 disambiguations, 52585 must
overlaps, 68893 queries
        aliasing_component_refs_p: 67104 disambiguations, 3081781 queries
        TBAA oracle: 22676608 disambiguations 61782455 queries
                     14044948 are in alias set 0
                     10998619 queries asked about the same object
                     153 queries asked about the same alias set
                     0 access volatile
                     12484882 are dependent in the DAG
                     1577245 are aritificially in conflict with void *

      Modref stats:
        modref kill: 832 kills, 19399 queries
        modref use: 50760 disambiguations, 1825106 queries
        modref clobber: 1371028 disambiguations, 40152504 queries
        5190319 tbaa queries (0.129265 per modref query)
        1341403 base compares (0.033408 per modref query)

      PTA query stats:
        pt_solution_includes: 36784449 disambiguations, 46141210 queries
        pt_solutions_intersect: 4519320 disambiguations, 17082083 queries

    gcc/ChangeLog:

    2023-12-13  Martin Jambor  <mjambor@suse.cz>

            PR tree-optimization/111807
            * tree-sra.cc (build_ref_for_model): Allow offset smaller than
            model->offset when gsi is non-NULL.  Adjust function comment.