[Bug tree-optimization/93820] [8/9/10 Regression] wrong code at -O3 on x86_64-linux-gnu since r9-1732

[cvs-commit at gcc dot gnu.org]

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

--- Comment #8 from CVS 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:4d213bf6011ed2b30b9d0ca70069a5dbc294b5d7

commit r10-6860-g4d213bf6011ed2b30b9d0ca70069a5dbc294b5d7
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Feb 26 09:33:48 2020 +0100

    store-merging: Fix coalesce_immediate_stores [PR93820]

    The following testcase is miscompiled in 8+.
    The problem is that check_no_overlap has a special case for INTEGER_CST
    marked stores (i.e. stores of constants), if both all currenly merged
stores
    and the one under consideration for merging with them are marked that way,
    it anticipates that other INTEGER_CST marked stores that overlap with those
    and precede those (have smaller info->order) could be merged with those and
    doesn't punt for them.
    In PR86844 and PR87859 fixes I've then added quite large code that is
    performed after check_no_overlap and tries to find out if we need and can
    merge further INTEGER_CST marked stores, or need to punt.
    Unfortunately, that code is there only in the overlapping case code and
    the testcase below shows that we really need it even in the adjacent store
    case.  After sort_by_bitpos we have:
    bitpos      width   order   rhs_code
    96  32      3       INTEGER_CST
    128 32      1       INTEGER_CST
    128 128     2       INTEGER_CST
    192 32      0       MEM_REF
    Because of the missing PR86844/PR87859-ish code in the adjacent store
    case, we merge the adjacent (memory wise) stores 96/32/3 and 128/32/1,
    and then we consider the 128-bit store which is in program-order in between
    them, but in this case we punt, because the merging would extend the
    merged store region from bitpos 96 and 64-bits to bitpos 96 and 160-bits
    and that has an overlap with an incompatible store (the MEM_REF one).
    The problem is that we can't really punt this way, because the 128-bit
    store is in between those two we've merged already, so either we manage
    to merge even that one together with the others, or would need to avoid
    already merging the 96/32/3 and 128/32/1 stores together.
    Now, rather than copying around the PR86844/PR87859 code to the other spot,
    we can actually just use the overlapping code, merge_overlapping is really
    a superset of merge_into, so that is what the patch does.  If doing
    adjacent store merge for rhs_code other than INTEGER_CST, I believe the
    current code is already fine, check_no_overlap in that case doesn't make
    the exception and will punt if there is some earlier (smaller order)
    non-mergeable overlapping store.  There is just one case that could be
    problematic, if the merged_store has BIT_INSERT_EXPRs in them and the
    new store is a constant store (INTEGER_CST rhs_code), then check_no_overlap
    would do the exception and still would allow the special case.  But we
    really shouldn't have the special case in that case, so this patch also
    changes check_no_overlap to just have a bool whether we should have the
    special case or not.

    Note, as I said in the PR, for GCC11 we could consider performing some kind
    of cheap DSE during the store merging (perhaps guarded with flag_tree_dse).
    And another thing to consider is only consider as problematic non-mergeable
    stores that not only have order smaller than last_order as currently, but
    also have order larger than first_order, as in this testcase if we actually
    ignored (not merged with anything at all) the 192/32/0 store, because it is
    not in between the other stores we'd merge, it would be fine to merge the
    other 3 stores, though of course the testcase can be easily adjusted by
    putting the 192/32 store after the 128/32 store and then this patch would
be
    still needed.  Though, I think I'd need more time thinking this over.

    2020-02-26  Jakub Jelinek  <jakub@redhat.com>

        PR tree-optimization/93820
        * gimple-ssa-store-merging.c (check_no_overlap): Change RHS_CODE
        argument to ALL_INTEGER_CST_P boolean.
        (imm_store_chain_info::try_coalesce_bswap): Adjust caller.
        (imm_store_chain_info::coalesce_immediate_stores): Likewise.  Handle
        adjacent INTEGER_CST store into merged_store->only_constants like
        overlapping one.

        * gcc.dg/pr93820.c: New test.