[Bug tree-optimization/84503] [7/8 Regression] store-merging miscompilation on powerpc64 with -O3 since r241789

[jakub at gcc dot gnu.org]

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

--- Comment #6 from Jakub Jelinek  ---
Author: jakub
Date: Thu Feb 22 08:28:42 2018
New Revision: 257891

URL: https://gcc.gnu.org/viewcvs?rev=257891&root=gcc&view=rev
Log:
PR tree-optimization/84503
* gimple-ssa-store-merging.c (merged_store_group::merge_into): Compute
width as info->bitpos + info->bitsize - start.
(merged_store_group::merge_overlapping): Simplify width computation.
(check_no_overlap): New function.
(imm_store_chain_info::try_coalesce_bswap): Compute expected
start + width and last_order of the group, fail if check_no_overlap
fails.
(imm_store_chain_info::coalesce_immediate_stores): Don't merge info
to group if check_no_overlap fails.

* gcc.dg/pr84503-1.c: New test.
* gcc.dg/pr84503-2.c: New test.

Added:
trunk/gcc/testsuite/gcc.dg/pr84503-1.c
trunk/gcc/testsuite/gcc.dg/pr84503-2.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/gimple-ssa-store-merging.c
trunk/gcc/testsuite/ChangeLog

[Bug tree-optimization/84503] [7/8 Regression] store-merging miscompilation on powerpc64 with -O3 since r241789

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek  ---
In 7.x this exact problem doesn't really exist, so the issue must be different
there.
I'd think it might be something fixable by PR82916 - like patch, but haven't
actually tried that yet.

[Bug tree-optimization/84503] [7/8 Regression] store-merging miscompilation on powerpc64 with -O3 since r241789

[marxin at gcc dot gnu.org]

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

--- Comment #4 from Martin Liška  ---
*** Bug 84505 has been marked as a duplicate of this bug. ***

[Bug tree-optimization/84503] [7/8 Regression] store-merging miscompilation on powerpc64 with -O3 since r241789

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek  ---
Created attachment 43485
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=43485&action=edit
gcc8-pr84503.patch

Untested fix.

[Bug tree-optimization/84503] [7/8 Regression] store-merging miscompilation on powerpc64 with -O3 since r241789

[jakub at gcc dot gnu.org]

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

--- Comment #2 from Jakub Jelinek  ---
The bug is in the way we handle overlapping stores.  The problem is that all we
do if there is overlap is:
  if (IN_RANGE (info->bitpos, merged_store->start,
merged_store->start + merged_store->width - 1))
{
  /* Only allow overlapping stores of constants.  */
  if (info->rhs_code == INTEGER_CST
  && merged_store->stores[0]->rhs_code == INTEGER_CST)
{
  merged_store->merge_overlapping (info);
  continue;
}
}
otherwise we:
  /* |---store 1---|  |---store 2---|.
 Gap between stores or the rhs not compatible.  Start a new group.  */

  /* Try to apply all the stores recorded for the group to determine
 the bitpattern they write and discard it if that fails.
 This will also reject single-store groups.  */
  if (!merged_store->apply_stores ())
delete merged_store;
  else
m_merged_store_groups.safe_push (merged_store);

  merged_store = new merged_store_group (info);
But the statements here are sorted primarily by bitpos and we emit statements
for the group at the location of the last statement (highest order) in the
merged store group.  So I think we need to check before adding another store if
it is not rhs_code INTEGER_CST, whether there is any overlap with following
stores.  Overlap is fine if the order of all the overlapping statements is
higher than MAX (merged_store->last_order, info->order), because then we know
we'll start a new group right after info and the merged stores of the current
group will come before any overlapping stores (whether merged successfully with
something or not), but otherwise we just shouldn't add info into the current
group.

[Bug tree-optimization/84503] [7/8 Regression] store-merging miscompilation on powerpc64 with -O3 since r241789

[jakub at gcc dot gnu.org]

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

--- Comment #1 from Jakub Jelinek  ---
Can be reproduced also on x86_64-linux with -O3 -fno-tree-vectorize
-fno-ivopts.

For the latter, I wonder what's the point in using TARGET_MEM_REF in:
  MEM[(long int *)p_28] = 0;
  MEM[(long int *)p_28 + 8B] = 0;
  MEM[(long int *)p_28 + 16B] = 0;
  MEM[(long int *)p_28 + 24B] = 0;
  MEM[(long int *)p_28 + 32B] = 0;
  MEM[(long int *)p_28 + 40B] = 0;
  MEM[(long int *)p_28 + 48B] = 0;
, isn't that something that MEM_REF can express too?  store-merging doesn't
handle TARGET_MEM_REFs and only handles MEM_REFs.  So, for stage1 shall it
handle also TARGET_MEM_REFs that only have base and optionally constant disp
and nothing else, or shall ivopts pass instead just generate MEM_REFs in those
cases?

[Bug tree-optimization/84503] [7/8 Regression] store-merging miscompilation on powerpc64 with -O3 since r241789

[jakub at gcc dot gnu.org]

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

Jakub Jelinek  changed:

   What|Removed |Added

   Priority|P3  |P2
 Status|UNCONFIRMED |ASSIGNED
   Last reconfirmed||2018-02-21
   Assignee|unassigned at gcc dot gnu.org  |jakub at gcc dot gnu.org
   Target Milestone|--- |8.0
 Ever confirmed|0   |1