[Bug middle-end/90348] [9/10/11/12 Regression] Partition of char arrays is incorrect in some cases

[Bug middle-end/90348] [9/10/11/12 Regression] Partition of char arrays is incorrect in some cases

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|8.5                         |9.4

--- Comment #22 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 8 branch is being closed.

[Bug middle-end/90348] [9/10/11/12 Regression] Partition of char arrays is incorrect in some cases

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|9.4                         |9.5

--- Comment #23 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 9.4 is being released, retargeting bugs to GCC 9.5.

[Bug middle-end/90348] [9/10/11/12 Regression] Partition of char arrays is incorrect in some cases

[rguenth at gcc dot gnu.org]

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

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

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

[Bug middle-end/90348] [9/10/11/12 Regression] Partition of char arrays is incorrect in some cases

[cvs-commit at gcc dot gnu.org]

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

--- Comment #24 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:551aa75778a4c5165d9533cd447c8fc822f583e1

commit r12-7044-g551aa75778a4c5165d9533cd447c8fc822f583e1
Author: Richard Biener <rguenther@suse.de>
Date:   Wed Feb 2 14:24:39 2022 +0100

    Add CLOBBER_EOL to mark storage end-of-life clobbers

    This adds a flag to CONSTRUCTOR nodes indicating that for
    clobbers this marks the end-of-life of storage as opposed to
    just ending the lifetime of the object that occupied it.
    The dangling pointer diagnostics uses CLOBBERs but is confused
    by those emitted by the C++ frontend for example which emits
    them for the second purpose at the start of CTORs.  The issue
    is also appearant for aarch64 in PR104092.

    Distinguishing the two cases is also necessary for the PR90348 fix.

    Since I'm going to add another flag I added an enum clobber_flags
    and a defaulted argument to build_clobber plus a convenient way to
    query the enum from the CTOR tree and specify it for gimple_clobber_p.
    Since 'CLOBBER' is already taken and I needed a name for the unspecified
    clobber we have now I used 'CLOBBER_UNDEF'.

    2022-02-03  Richard Biener  <rguenther@suse.de>

            PR middle-end/90348
            PR middle-end/104092
    gcc/
            * tree-core.h (clobber_kind): New enum.
            (tree_base::u::bits::address_space): Document use in CONSTRUCTORs.
            * tree.h (CLOBBER_KIND): Add.
            (build_clobber): Add clobber kind argument, defaulted to
            CLOBBER_UNDEF.
            * tree.cc (build_clobber): Likewise.
            * gimple.h (gimple_clobber_p): New overload with specified kind.
            * tree-streamer-in.cc (streamer_read_tree_bitfields): Stream
            CLOBBER_KIND.
            * tree-streamer-out.cc (streamer_write_tree_bitfields):
            Likewise.
            * tree-pretty-print.cc (dump_generic_node): Mark EOL CLOBBERs.
            * gimplify.cc (gimplify_bind_expr): Build storage end-of-life
clobbers
            with CLOBBER_EOL.
            (gimplify_target_expr): Likewise.
            * tree-inline.cc (expand_call_inline): Likewise.
            * tree-ssa-ccp.cc (insert_clobber_before_stack_restore): Likewise.
            * gimple-ssa-warn-access.cc (pass_waccess::check_stmt): Only treat
            CLOBBER_EOL clobbers as ending lifetime of storage.

    gcc/lto/
            * lto-common.cc (compare_tree_sccs_1): Compare CLOBBER_KIND.

    gcc/testsuite/
            * gcc.dg/pr87052.c: Adjust.

[Bug middle-end/90348] [10/11/12/13 Regression] Partition of char arrays is incorrect in some cases

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|9.5                         |10.4

--- Comment #25 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 9 branch is being closed

[Bug middle-end/90348] [10/11/12/13 Regression] Partition of char arrays is incorrect in some cases

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.4                        |10.5

--- Comment #26 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
GCC 10.4 is being released, retargeting bugs to GCC 10.5.

[Bug middle-end/90348] [10/11/12/13/14 Regression] Partition of char arrays is incorrect in some cases

[rguenth at gcc dot gnu.org]

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

--- Comment #27 from Richard Biener <rguenth at gcc dot gnu.org> ---
The testcase in comment#2 still reproduces on the GCC 11 branch but no longer
with GCC 12+ (on x86_64-linux, -O2).

[Bug middle-end/90348] [11/12/13/14 Regression] Partition of char arrays is incorrect in some cases

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|10.5                        |11.5

--- Comment #28 from Richard Biener <rguenth at gcc dot gnu.org> ---
GCC 10 branch is being closed.

[Bug middle-end/90348] [11/12/13/14 Regression] Partition of char arrays is incorrect in some cases

[gnu.ojxq8 at dralias dot com]

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

maic <gnu.ojxq8 at dralias dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |gnu.ojxq8 at dralias dot com

--- Comment #29 from maic <gnu.ojxq8 at dralias dot com> ---
> GCC 11 branch but no longer with GCC 12+

So is this fixed and can be closed? (Also, the title could be edited then to
remove gcc 12-14)

[Bug middle-end/90348] [11/12/13/14 Regression] Partition of char arrays is incorrect in some cases

[rguenther at suse dot de]

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

--- Comment #30 from rguenther at suse dot de <rguenther at suse dot de> ---
On Wed, 19 Jul 2023, gnu.ojxq8 at dralias dot com wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90348
> 
> maic <gnu.ojxq8 at dralias dot com> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>                  CC|                            |gnu.ojxq8 at dralias dot com
> 
> --- Comment #29 from maic <gnu.ojxq8 at dralias dot com> ---
> > GCC 11 branch but no longer with GCC 12+
> 
> So is this fixed and can be closed? (Also, the title could be edited then to
> remove gcc 12-14)

No it's not fixed, just the testcase doesn't expose the issue anymore.

[Bug middle-end/90348] [11/12/13/14 Regression] Partition of char arrays is incorrect in some cases

[cvs-commit at gcc dot gnu.org]

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

--- Comment #31 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:1251d3957de04dc9b023a23c09400217e13deadb

commit r14-7274-g1251d3957de04dc9b023a23c09400217e13deadb
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Jan 16 11:49:34 2024 +0100

    cfgexpand: Workaround CSE of ADDR_EXPRs in VAR_DECL partitioning [PR113372]

    The following patch adds a quick workaround to bugs in VAR_DECL
    partitioning.
    The problem is that there is no dependency between ADDR_EXPRs of local
    decls and CLOBBERs of those vars, so VN can CSE uses of ADDR_EXPRs
    (including ivopts integral variants thereof), which can break
    add_scope_conflicts discovery of what variables are actually used
    in certain region.
    E.g. we can have
      ivtmp.40_3 = (unsigned long) &MEM <unsigned long[100]> [(void *)&bitint.6
+ 8B];
    ...
      uses of ivtmp.40_3
    ...
      bitint.6 ={v} {CLOBBER(eos)};
    ...
      ivtmp.28_43 = (unsigned long) &MEM <unsigned long[100]> [(void
*)&bitint.6 + 8B];
    ...
      uses of ivtmp.28_43
    before VN (such as dom3), which the add_scope_conflicts code identifies as
2
    independent uses of bitint.6 variable (which is correct), but then VN
    determines ivtmp.28_43 is the same as ivtmp.40_3 and just uses ivtmp.40_3
    even in the second region; at that point add_scope_conflict thinks the
    bitint.6 variable is not used in that region anymore.

    The following patch does a simple single def-stmt check for such ADDR_EXPRs
    (rather than say trying to do a full propagation of what SSA_NAMEs can
    contain ADDR_EXPRs of local variables), which seems to workaround all 4
PRs.

    In addition to this patch I've used the attached one to gather statistics
    on the total size of all variable partitions in a function and seems
besides
    the new testcases nothing is really affected compared to no patch (I've
    actually just modified the patch to == OMP_SCAN instead of == ADDR_EXPR, so
    it looks the same except that it never triggers).  The comparison wasn't
    perfect because I've only gathered BITS_PER_WORD, main_input_filename (did
    some replacement of build directories and /tmp/ccXXXXXX names of LTO to
make
    it more similar between the two bootstraps/regtests), current_function_name
    and the total size of all variable partitions if any, because I didn't
    record e.g. the optimization options and so e.g. torture tests which
iterate
    over options could have different partition sizes even in one compiler when
    BITS_PER_WORD, main_input_filename and current_function_name are all equal.
    So had to write an awk script to check if the first triple in the second
    build appeared in the first one and the quadruple in the second build
    appeared in the first one too, otherwise print result and that only
    triggered in the new tests.
    Also, the cc1plus binary according to objdump -dr is identical between the
    two builds except for the ADDR_EXPR vs. OMP_SCAN constant in the two spots.

    2024-01-16  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/113372
            PR middle-end/90348
            PR middle-end/110115
            PR middle-end/111422
            * cfgexpand.cc (add_scope_conflicts_2): New function.
            (add_scope_conflicts_1): Use it.

            * gcc.dg/torture/bitint-49.c: New test.
            * gcc.c-torture/execute/pr90348.c: New test.
            * gcc.c-torture/execute/pr110115.c: New test.
            * gcc.c-torture/execute/pr111422.c: New test.

[Bug middle-end/90348] [11/12/13/14 Regression] Partition of char arrays is incorrect in some cases

[cvs-commit at gcc dot gnu.org]

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

--- Comment #32 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:432708c306838fe1444da0df7d629a60468c0c73

commit r13-8383-g432708c306838fe1444da0df7d629a60468c0c73
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Tue Jan 16 11:49:34 2024 +0100

    cfgexpand: Workaround CSE of ADDR_EXPRs in VAR_DECL partitioning [PR113372]

    The following patch adds a quick workaround to bugs in VAR_DECL
    partitioning.
    The problem is that there is no dependency between ADDR_EXPRs of local
    decls and CLOBBERs of those vars, so VN can CSE uses of ADDR_EXPRs
    (including ivopts integral variants thereof), which can break
    add_scope_conflicts discovery of what variables are actually used
    in certain region.
    E.g. we can have
      ivtmp.40_3 = (unsigned long) &MEM <unsigned long[100]> [(void *)&bitint.6
+ 8B];
    ...
      uses of ivtmp.40_3
    ...
      bitint.6 ={v} {CLOBBER(eos)};
    ...
      ivtmp.28_43 = (unsigned long) &MEM <unsigned long[100]> [(void
*)&bitint.6 + 8B];
    ...
      uses of ivtmp.28_43
    before VN (such as dom3), which the add_scope_conflicts code identifies as
2
    independent uses of bitint.6 variable (which is correct), but then VN
    determines ivtmp.28_43 is the same as ivtmp.40_3 and just uses ivtmp.40_3
    even in the second region; at that point add_scope_conflict thinks the
    bitint.6 variable is not used in that region anymore.

    The following patch does a simple single def-stmt check for such ADDR_EXPRs
    (rather than say trying to do a full propagation of what SSA_NAMEs can
    contain ADDR_EXPRs of local variables), which seems to workaround all 4
PRs.

    In addition to this patch I've used the attached one to gather statistics
    on the total size of all variable partitions in a function and seems
besides
    the new testcases nothing is really affected compared to no patch (I've
    actually just modified the patch to == OMP_SCAN instead of == ADDR_EXPR, so
    it looks the same except that it never triggers).  The comparison wasn't
    perfect because I've only gathered BITS_PER_WORD, main_input_filename (did
    some replacement of build directories and /tmp/ccXXXXXX names of LTO to
make
    it more similar between the two bootstraps/regtests), current_function_name
    and the total size of all variable partitions if any, because I didn't
    record e.g. the optimization options and so e.g. torture tests which
iterate
    over options could have different partition sizes even in one compiler when
    BITS_PER_WORD, main_input_filename and current_function_name are all equal.
    So had to write an awk script to check if the first triple in the second
    build appeared in the first one and the quadruple in the second build
    appeared in the first one too, otherwise print result and that only
    triggered in the new tests.
    Also, the cc1plus binary according to objdump -dr is identical between the
    two builds except for the ADDR_EXPR vs. OMP_SCAN constant in the two spots.

    2024-01-16  Jakub Jelinek  <jakub@redhat.com>

            PR tree-optimization/113372
            PR middle-end/90348
            PR middle-end/110115
            PR middle-end/111422
            * cfgexpand.cc (add_scope_conflicts_2): New function.
            (add_scope_conflicts_1): Use it.

            * gcc.c-torture/execute/pr90348.c: New test.
            * gcc.c-torture/execute/pr110115.c: New test.
            * gcc.c-torture/execute/pr111422.c: New test.

    (cherry picked from commit 1251d3957de04dc9b023a23c09400217e13deadb)

[Bug middle-end/90348] [11/12 Regression] Partition of char arrays is incorrect in some cases

[jakub at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[11/12/13/14 Regression]    |[11/12 Regression]
                   |Partition of char arrays is |Partition of char arrays is
                   |incorrect in some cases     |incorrect in some cases

--- Comment #33 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Worked around for GCC 13.3+ and 14+.