This enhances store-order preserving store motion to handle the case
of non-invariant dependent stores in the sequence of unconditionally
executed stores on exit by re-issueing them as part of the sequence
of stores on the exit. This fixes the observed regression of
gcc.target/i386/pr64110.c which relies on store-motion of 'b'
for a loop like
for (int i = 0; i < j; ++i)
*b++ = x;
where for correctness we now no longer apply store-motion. With
the patch we emit the correct
tem = b;
for (int i = 0; i < j; ++i)
{
tem = tem + 1;
*tem = x;
}
b = tem;
*tem = x;
preserving the original order of stores. A testcase reflecting
the miscompilation done by earlier GCC is added as well.
2020-05-08 Richard Biener <rguent...@suse.de>
PR tree-optimization/94988
* tree-ssa-loop-im.c (seq_entry): Make a struct, add from.
(execute_sm_exit): Re-issue sm_other stores in the correct
order.
(sm_seq_valid_bb): When always executed, allow sm_other to
prevail inbetween sm_ord and record their stored value.
(hoist_memory_references): Adjust refs_not_supported propagation
and prune sm_other from the end of the ordered sequences.
* gcc.dg/torture/pr94988.c: New testcase.
---
gcc/testsuite/gcc.dg/torture/pr94988.c | 20 +++++++++
gcc/tree-ssa-loop-im.c | 75 ++++++++++++++++++++++++++--------
2 files changed, 78 insertions(+), 17 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/torture/pr94988.c
diff --git a/gcc/testsuite/gcc.dg/torture/pr94988.c
b/gcc/testsuite/gcc.dg/torture/pr94988.c
new file mode 100644
index 00000000000..1ee99fea5ce
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr94988.c
@@ -0,0 +1,20 @@
+/* { dg-do run } */
+
+short *b;
+
+void __attribute__((noipa))
+bar (short x, int j)
+{
+ for (int i = 0; i < j; ++i)
+ *b++ = x;
+}
+
+int
+main()
+{
+ b = (short *)&b;
+ bar (0, 1);
+ if ((short)(__UINTPTR_TYPE__)b != 0)
+ __builtin_abort ();
+ return 0;
+}
diff --git a/gcc/tree-ssa-loop-im.c b/gcc/tree-ssa-loop-im.c
index 2aabb54c98d..9a9f289738d 100644
--- a/gcc/tree-ssa-loop-im.c
+++ b/gcc/tree-ssa-loop-im.c
@@ -2209,7 +2209,14 @@ execute_sm (class loop *loop, im_mem_ref *ref,
able to execute in arbitrary order with respect to other stores
sm_other is used for stores we do not try to apply store motion to. */
enum sm_kind { sm_ord, sm_unord, sm_other };
-typedef std::pair<unsigned, sm_kind> seq_entry;
+struct seq_entry
+{
+ seq_entry (unsigned f, sm_kind k, tree fr = NULL)
+ : first (f), second (k), from (fr) {}
+ unsigned first;
+ sm_kind second;
+ tree from;
+};
static void
execute_sm_exit (class loop *loop, edge ex, vec<seq_entry> &seq,
@@ -2218,20 +2225,36 @@ execute_sm_exit (class loop *loop, edge ex,
vec<seq_entry> &seq,
/* Sink the stores to exit from the loop. */
for (unsigned i = seq.length (); i > 0; --i)
{
- if (seq[i-1].second != kind)
- continue;
im_mem_ref *ref = memory_accesses.refs_list[seq[i-1].first];
- sm_aux *aux = *aux_map.get (ref);
- if (!aux->store_flag)
+ if (seq[i-1].second == sm_other)
{
- gassign *store;
- store = gimple_build_assign (unshare_expr (ref->mem.ref),
- aux->tmp_var);
+ gcc_assert (kind == sm_ord && seq[i-1].from != NULL_TREE);
+ if (dump_file && (dump_flags & TDF_DETAILS))
+ {
+ fprintf (dump_file, "Re-issueing dependent store of ");
+ print_generic_expr (dump_file, ref->mem.ref);
+ fprintf (dump_file, " from loop %d on exit %d -> %d\n",
+ loop->num, ex->src->index, ex->dest->index);
+ }
+ gassign *store = gimple_build_assign (unshare_expr (ref->mem.ref),
+ seq[i-1].from);
gsi_insert_on_edge (ex, store);
}
else
- execute_sm_if_changed (ex, ref->mem.ref, aux->tmp_var, aux->store_flag,
- loop_preheader_edge (loop), &aux->flag_bbs);
+ {
+ sm_aux *aux = *aux_map.get (ref);
+ if (!aux->store_flag)
+ {
+ gassign *store;
+ store = gimple_build_assign (unshare_expr (ref->mem.ref),
+ aux->tmp_var);
+ gsi_insert_on_edge (ex, store);
+ }
+ else
+ execute_sm_if_changed (ex, ref->mem.ref, aux->tmp_var,
+ aux->store_flag,
+ loop_preheader_edge (loop), &aux->flag_bbs);
+ }
}
}
@@ -2429,12 +2452,16 @@ sm_seq_valid_bb (class loop *loop, basic_block bb, tree
vdef,
/* One of the stores we want to apply SM to and we've not yet seen. */
else if (bitmap_clear_bit (refs_not_in_seq, data->ref))
{
- seq.safe_push (std::make_pair (data->ref, sm_ord));
+ seq.safe_push (seq_entry (data->ref, sm_ord));
/* 1) push it down the queue until a SMed
and not ignored ref is reached, skipping all not SMed refs
and ignored refs via non-TBAA disambiguation. */
- if (!sm_seq_push_down (seq, seq.length () - 1))
+ if (!sm_seq_push_down (seq, seq.length () - 1)
+ /* If that fails but we did not fork yet continue, we'll see
+ to re-materialize all of the stores in the sequence then.
+ Further stores will only be pushed up to this one. */
+ && forked)
{
bitmap_set_bit (refs_not_supported, data->ref);
/* ??? Mark it sm_unord but it's now "somewhere" ... */
@@ -2453,7 +2480,8 @@ sm_seq_valid_bb (class loop *loop, basic_block bb, tree
vdef,
}
else
/* Another store not part of the final sequence. Simply push it. */
- seq.safe_push (std::make_pair (data->ref, sm_other));
+ seq.safe_push (seq_entry (data->ref, sm_other,
+ gimple_assign_rhs1 (def)));
vdef = gimple_vuse (def);
}
@@ -2513,14 +2541,21 @@ hoist_memory_references (class loop *loop, bitmap
mem_refs,
std::pair<edge, vec<seq_entry> > *seq;
FOR_EACH_VEC_ELT (sms, i, seq)
{
+ bool need_to_push = false;
for (unsigned i = 0; i < seq->second.length (); ++i)
{
- if (seq->second[i].second == sm_other)
+ if (seq->second[i].second == sm_other
+ && seq->second[i].from == NULL_TREE)
break;
unsigned id = seq->second[i].first;
if (bitmap_bit_p (refs_not_supported, id))
- seq->second[i].second = sm_other;
- else if (!sm_seq_push_down (seq->second, i))
+ {
+ seq->second[i].second = sm_other;
+ seq->second[i].from = NULL_TREE;
+ need_to_push = true;
+ }
+ else if (need_to_push
+ && !sm_seq_push_down (seq->second, i))
{
if (bitmap_set_bit (refs_not_supported, id))
changed = true;
@@ -2528,6 +2563,12 @@ hoist_memory_references (class loop *loop, bitmap
mem_refs,
}
}
}
+ /* Prune sm_other from the end. */
+ std::pair<edge, vec<seq_entry> > *seq;
+ FOR_EACH_VEC_ELT (sms, i, seq)
+ while (!seq->second.is_empty ()
+ && seq->second.last ().second == sm_other)
+ seq->second.pop ();
/* Verify dependence for refs we cannot handle with the order preserving
code (refs_not_supported) or prune them from mem_refs. */
@@ -2540,7 +2581,7 @@ hoist_memory_references (class loop *loop, bitmap
mem_refs,
/* We've now verified store order for ref with respect to all other
stores in the loop does not matter. */
else
- unord_refs.safe_push (std::make_pair (i, sm_unord));
+ unord_refs.safe_push (seq_entry (i, sm_unord));
}
hash_map<im_mem_ref *, sm_aux *> aux_map;
--
2.13.7
