When doing whole-function SLP we have to make sure the recorded
base alignments we compute as the maximum alignment seen for a
base anywhere in the function is actually valid at the point
we want to make use of it.

To make this work we now record the stmt the alignment was derived
from in addition to the DRs innermost behavior and we use a
dominance check to verify the recorded info is valid when doing
BB vectorization.

Note this leaves a small(?) hole for the case where we have sth
like

    unaligned DR
    call (); // does not return
    aligned DR

since we'll derive an aligned access for the earlier DR but the
later DR is never actually reached since the call does not
return.  To plug this hole one option (for the easy backporting)
would be to simply not use the base-alignment recording at all.
Alternatively we'd have to store the dataref grouping 'id' somewhere
in the DR itself and use that to handle this particular case.

For optimal handling we'd need the ability to record different
base alignments based on context, we could hash on the BB and
at query time walk immediate dominators to find the "best"
base alignment.  But I'd rather leave such improvement for trunk.

Any opinions?  The issue looks quite serious and IMHO warrants a
timely fix, even if partial - I'm not sure how often the same-BB
case would trigger.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Thanks,
Richard.

2021-08-31  Richard Biener  <rguent...@suse.de>

        PR tree-optimization/102139
        * tree-vectorizer.h (vec_base_alignments): Adjust hash-map
        type to record a std::pair of the stmt and the innermost
        loop behavior.
        * tree-vect-data-refs.c (vect_record_base_alignment): Adjust.
        (vect_compute_data_ref_alignment): Verify the recorded
        base alignment can be used.
---
 gcc/tree-vect-data-refs.c | 19 ++++++++++++-------
 gcc/tree-vectorizer.h     |  7 ++++---
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
index 37f46d1aaa3..e2549811961 100644
--- a/gcc/tree-vect-data-refs.c
+++ b/gcc/tree-vect-data-refs.c
@@ -895,11 +895,11 @@ vect_record_base_alignment (vec_info *vinfo, 
stmt_vec_info stmt_info,
                            innermost_loop_behavior *drb)
 {
   bool existed;
-  innermost_loop_behavior *&entry
+  std::pair<gimple *, innermost_loop_behavior *> &entry
     = vinfo->base_alignments.get_or_insert (drb->base_address, &existed);
-  if (!existed || entry->base_alignment < drb->base_alignment)
+  if (!existed || entry.second->base_alignment < drb->base_alignment)
     {
-      entry = drb;
+      entry = std::make_pair (stmt_info->stmt, drb);
       if (dump_enabled_p ())
        dump_printf_loc (MSG_NOTE, vect_location,
                         "recording new base alignment for %T\n"
@@ -1060,11 +1060,16 @@ vect_compute_data_ref_alignment (vec_info *vinfo, 
dr_vec_info *dr_info)
 
   /* Calculate the maximum of the pooled base address alignment and the
      alignment that we can compute for DR itself.  */
-  innermost_loop_behavior **entry = base_alignments->get (drb->base_address);
-  if (entry && base_alignment < (*entry)->base_alignment)
+  std::pair<gimple *, innermost_loop_behavior *> *entry
+    = base_alignments->get (drb->base_address);
+  if (entry
+      && base_alignment < (*entry).second->base_alignment
+      && (loop_vinfo
+         || dominated_by_p (CDI_DOMINATORS, gimple_bb (stmt_info->stmt),
+                            gimple_bb (entry->first))))
     {
-      base_alignment = (*entry)->base_alignment;
-      base_misalignment = (*entry)->base_misalignment;
+      base_alignment = entry->second->base_alignment;
+      base_misalignment = entry->second->base_misalignment;
     }
 
   if (drb->offset_alignment < vect_align_c
diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
index 72e018e8eac..8db642c7dc3 100644
--- a/gcc/tree-vectorizer.h
+++ b/gcc/tree-vectorizer.h
@@ -106,10 +106,11 @@ struct stmt_info_for_cost {
 
 typedef vec<stmt_info_for_cost> stmt_vector_for_cost;
 
-/* Maps base addresses to an innermost_loop_behavior that gives the maximum
-   known alignment for that base.  */
+/* Maps base addresses to an innermost_loop_behavior and the stmt it was
+   derived from that gives the maximum known alignment for that base.  */
 typedef hash_map<tree_operand_hash,
-                innermost_loop_behavior *> vec_base_alignments;
+                std::pair<gimple *, innermost_loop_behavior *> >
+         vec_base_alignments;
 
 /************************************************************************
   SLP
-- 
2.31.1

Reply via email to