diff mbox series

[vect] Keep track of DR_OFFSET advance in dr_vec_info rather than data_reference

Message ID 7bff5728-7991-80fc-d630-1e344f810699@arm.com
State New
Headers show
Series [vect] Keep track of DR_OFFSET advance in dr_vec_info rather than data_reference | expand

Commit Message

Andre Vieira (lists) Dec. 10, 2019, 1:36 p.m. UTC 
Hi,

This patch aims at refactoring the vectorizer code to no longer need to 
reset DR_OFFSET for epilogue vectorization and instead keep track of 
DR_OFFSET changes per dr_vec_info and just update it as needed.  I added 
a member of type 'tree' called 'offset' to dr_vec_info, which keeps 
track of the changes to the data_reference's offset per dr_vec_info and 
thus per instance of loop vectorization.  To get the current loop's 
DR_OFFSET I introduced a function 'get_dr_vinfo_offset' which will add 
the dr_vec_info's offset to either the data_reference's innermost offset 
or the offset of the 'innermost_loop_behavior' returned by 
'vect_dr_behavior' depending on whether 'get_dr_vinfo_offset's second 
parameter 'check_outer' is true.  This is to mimic the behavior of using 
the outer loop relative 'innermost_loop_behavior' in 
'vect_create_addr_base_for_vector_ref'.

I regression tested vect.exp on aarch64 and x86_64. I also regression 
tested libgomp on aarch64 and x86_64, no changes, but there were quite a 
few test failures with the commit I based this patch on...

Is this OK for trunk or shall I wait until libgomp stabilizes?

2019-12-10  Andre Vieira  <andre.simoesdiasvieira@arm.com>

         * tree-vect-data-refs.c (vect_create_addr_base_for_vector_ref): Use
         get_dr_vinfo_offset
         * tree-vect-loop.c (update_epilogue_loop_vinfo):  Remove 
orig_drs_init
         parameter and its use to reset DR_OFFSET's.
         (vect_transform_loop): Remove orig_drs_init argument.
         * tree-vect-loop-manip.c (vect_update_init_of_dr): Update the 
offset
         member of dr_vec_info rather than the offset of the associated
         data_reference's innermost_loop_behavior.
         (vect_update_init_of_dr): Pass dr_vec_info instead of 
data_reference.
         (vect_do_peeling): Remove orig_drs_init parameter and its
         construction.
         * tree-vect-stmts.c (check_scan_store): Replace use of DR_OFFSET
         with get_dr_vinfo_offset.
         (vectorizable_store): Likewise.
         (vectorizable_load): Likewise.

Comments

Andre Vieira (lists) Jan. 7, 2020, 1:35 p.m. UTC | #1 
Hello,

Should we try to get this refactoring in stage 3 or park it for next 
stage 1?

Cheers,
Andre

On 10/12/2019 13:36, Andre Vieira (lists) wrote:
> Hi,
> 
> This patch aims at refactoring the vectorizer code to no longer need to 
> reset DR_OFFSET for epilogue vectorization and instead keep track of 
> DR_OFFSET changes per dr_vec_info and just update it as needed.  I added 
> a member of type 'tree' called 'offset' to dr_vec_info, which keeps 
> track of the changes to the data_reference's offset per dr_vec_info and 
> thus per instance of loop vectorization.  To get the current loop's 
> DR_OFFSET I introduced a function 'get_dr_vinfo_offset' which will add 
> the dr_vec_info's offset to either the data_reference's innermost offset 
> or the offset of the 'innermost_loop_behavior' returned by 
> 'vect_dr_behavior' depending on whether 'get_dr_vinfo_offset's second 
> parameter 'check_outer' is true.  This is to mimic the behavior of using 
> the outer loop relative 'innermost_loop_behavior' in 
> 'vect_create_addr_base_for_vector_ref'.
> 
> I regression tested vect.exp on aarch64 and x86_64. I also regression 
> tested libgomp on aarch64 and x86_64, no changes, but there were quite a 
> few test failures with the commit I based this patch on...
> 
> Is this OK for trunk or shall I wait until libgomp stabilizes?
> 
> 2019-12-10  Andre Vieira  <andre.simoesdiasvieira@arm.com>
> 
>          * tree-vect-data-refs.c (vect_create_addr_base_for_vector_ref): 
> Use
>          get_dr_vinfo_offset
>          * tree-vect-loop.c (update_epilogue_loop_vinfo):  Remove 
> orig_drs_init
>          parameter and its use to reset DR_OFFSET's.
>          (vect_transform_loop): Remove orig_drs_init argument.
>          * tree-vect-loop-manip.c (vect_update_init_of_dr): Update the 
> offset
>          member of dr_vec_info rather than the offset of the associated
>          data_reference's innermost_loop_behavior.
>          (vect_update_init_of_dr): Pass dr_vec_info instead of 
> data_reference.
>          (vect_do_peeling): Remove orig_drs_init parameter and its
>          construction.
>          * tree-vect-stmts.c (check_scan_store): Replace use of DR_OFFSET
>          with get_dr_vinfo_offset.
>          (vectorizable_store): Likewise.
>          (vectorizable_load): Likewise.
Richard Biener Jan. 8, 2020, 7:38 a.m. UTC | #2 
On Tue, 7 Jan 2020, Andre Vieira (lists) wrote:

> Hello,
> 
> Should we try to get this refactoring in stage 3 or park it for next stage 1?

I only looked at the patch now and I think it nicely simplifies things.
The patch is OK now with the usual function comment added before

 }

+inline tree
+get_dr_vinfo_offset (dr_vec_info *dr_info, bool check_outer = false)
+{
+  innermost_loop_behavior *base;

Thanks,
Richard.

> Cheers,
> Andre
> 
> On 10/12/2019 13:36, Andre Vieira (lists) wrote:
> > Hi,
> > 
> > This patch aims at refactoring the vectorizer code to no longer need to
> > reset DR_OFFSET for epilogue vectorization and instead keep track of
> > DR_OFFSET changes per dr_vec_info and just update it as needed.  I added a
> > member of type 'tree' called 'offset' to dr_vec_info, which keeps track of
> > the changes to the data_reference's offset per dr_vec_info and thus per
> > instance of loop vectorization.  To get the current loop's DR_OFFSET I
> > introduced a function 'get_dr_vinfo_offset' which will add the dr_vec_info's
> > offset to either the data_reference's innermost offset or the offset of the
> > 'innermost_loop_behavior' returned by 'vect_dr_behavior' depending on
> > whether 'get_dr_vinfo_offset's second parameter 'check_outer' is true.  This
> > is to mimic the behavior of using the outer loop relative
> > 'innermost_loop_behavior' in 'vect_create_addr_base_for_vector_ref'.
> > 
> > I regression tested vect.exp on aarch64 and x86_64. I also regression tested
> > libgomp on aarch64 and x86_64, no changes, but there were quite a few test
> > failures with the commit I based this patch on...
> > 
> > Is this OK for trunk or shall I wait until libgomp stabilizes?
> > 
> > 2019-12-10  Andre Vieira  <andre.simoesdiasvieira@arm.com>
> > 
> >         * tree-vect-data-refs.c (vect_create_addr_base_for_vector_ref): Use
> >          get_dr_vinfo_offset
> >          * tree-vect-loop.c (update_epilogue_loop_vinfo):  Remove 
> > orig_drs_init
> >          parameter and its use to reset DR_OFFSET's.
> >          (vect_transform_loop): Remove orig_drs_init argument.
> >          * tree-vect-loop-manip.c (vect_update_init_of_dr): Update the 
> > offset
> >          member of dr_vec_info rather than the offset of the associated
> >          data_reference's innermost_loop_behavior.
> >          (vect_update_init_of_dr): Pass dr_vec_info instead of 
> > data_reference.
> >          (vect_do_peeling): Remove orig_drs_init parameter and its
> >          construction.
> >          * tree-vect-stmts.c (check_scan_store): Replace use of DR_OFFSET
> >          with get_dr_vinfo_offset.
> >          (vectorizable_store): Likewise.
> >          (vectorizable_load): Likewise.
> 
>
diff mbox series

Patch

diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
index b876d07f44b9922be478fe04b9e7cb201f245b24..6745f99e3e25f690ef80433bc2b0651e4b994ba2 100644
--- a/gcc/tree-vect-data-refs.c
+++ b/gcc/tree-vect-data-refs.c
@@ -4597,7 +4597,7 @@  vect_create_addr_base_for_vector_ref (stmt_vec_info stmt_info,
   innermost_loop_behavior *drb = vect_dr_behavior (dr_info);
 
   tree data_ref_base = unshare_expr (drb->base_address);
-  tree base_offset = unshare_expr (drb->offset);
+  tree base_offset = unshare_expr (get_dr_vinfo_offset (dr_info, true));
   tree init = unshare_expr (drb->init);
 
   if (loop_vinfo)
diff --git a/gcc/tree-vect-loop-manip.c b/gcc/tree-vect-loop-manip.c
index b4dda971b18e17733b121038e89e95fc59f09d0c..3cc76c30af50967a579164cc03c2a2cf2ba96640 100644
--- a/gcc/tree-vect-loop-manip.c
+++ b/gcc/tree-vect-loop-manip.c
@@ -1711,19 +1711,22 @@  vect_gen_prolog_loop_niters (loop_vec_info loop_vinfo,
    iterations before the scalar one (using masking to skip inactive
    elements).  This function updates the information recorded in DR to
    account for the difference.  Specifically, it updates the OFFSET
-   field of DR.  */
+   field of DR_INFO.  */
 
 static void
-vect_update_init_of_dr (struct data_reference *dr, tree niters, tree_code code)
+vect_update_init_of_dr (dr_vec_info *dr_info, tree niters, tree_code code)
 {
-  tree offset = DR_OFFSET (dr);
+  struct data_reference *dr = dr_info->dr;
+  tree offset = dr_info->offset;
+  if (!offset)
+    offset = build_zero_cst (sizetype);
 
   niters = fold_build2 (MULT_EXPR, sizetype,
 			fold_convert (sizetype, niters),
 			fold_convert (sizetype, DR_STEP (dr)));
   offset = fold_build2 (code, sizetype,
 			fold_convert (sizetype, offset), niters);
-  DR_OFFSET (dr) = offset;
+  dr_info->offset = offset;
 }
 
 
@@ -1753,7 +1756,7 @@  vect_update_inits_of_drs (loop_vec_info loop_vinfo, tree niters,
     {
       dr_vec_info *dr_info = loop_vinfo->lookup_dr (dr);
       if (!STMT_VINFO_GATHER_SCATTER_P (dr_info->stmt))
-	vect_update_init_of_dr (dr, niters, code);
+	vect_update_init_of_dr (dr_info, niters, code);
     }
 }
 
@@ -2441,7 +2444,7 @@  vect_do_peeling (loop_vec_info loop_vinfo, tree niters, tree nitersm1,
 		 tree *niters_vector, tree *step_vector,
 		 tree *niters_vector_mult_vf_var, int th,
 		 bool check_profitability, bool niters_no_overflow,
-		 tree *advance, drs_init_vec &orig_drs_init)
+		 tree *advance)
 {
   edge e, guard_e;
   tree type = TREE_TYPE (niters), guard_cond;
@@ -2656,14 +2659,6 @@  vect_do_peeling (loop_vec_info loop_vinfo, tree niters, tree nitersm1,
 	  scale_loop_profile (prolog, prob_prolog, bound_prolog);
 	}
 
-      /* Save original inits for each data_reference before advancing them with
-	 NITERS_PROLOG.  */
-      unsigned int i;
-      struct data_reference *dr;
-      vec<data_reference_p> datarefs = loop_vinfo->shared->datarefs;
-      FOR_EACH_VEC_ELT (datarefs, i, dr)
-	orig_drs_init.safe_push (std::make_pair (dr, DR_OFFSET (dr)));
-
       /* Update init address of DRs.  */
       vect_update_inits_of_drs (loop_vinfo, niters_prolog, PLUS_EXPR);
       /* Update niters for vector loop.  */
diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
index 83b0e0b8ce9c6c0979e64448ed9118587f0ae7a9..514af118ea347c8e3ff829aa034e9ba7d89e95e6 100644
--- a/gcc/tree-vect-loop.c
+++ b/gcc/tree-vect-loop.c
@@ -8315,8 +8315,7 @@  find_in_mapping (tree t, void *context)
    prologue of the main loop.  */
 
 static void
-update_epilogue_loop_vinfo (class loop *epilogue, tree advance,
-			    drs_init_vec &orig_drs_init)
+update_epilogue_loop_vinfo (class loop *epilogue, tree advance)
 {
   loop_vec_info epilogue_vinfo = loop_vec_info_for_loop (epilogue);
   auto_vec<gimple *> stmt_worklist;
@@ -8326,16 +8325,10 @@  update_epilogue_loop_vinfo (class loop *epilogue, tree advance,
   gphi_iterator epilogue_phi_gsi;
   stmt_vec_info stmt_vinfo = NULL, related_vinfo;
   basic_block *epilogue_bbs = get_loop_body (epilogue);
+  unsigned i;
 
   LOOP_VINFO_BBS (epilogue_vinfo) = epilogue_bbs;
 
-  /* Restore original data_reference's offset, before the previous loop and its
-     prologue.  */
-  std::pair<data_reference*, tree> *dr_init;
-  unsigned i;
-  for (i = 0; orig_drs_init.iterate (i, &dr_init); i++)
-    DR_OFFSET (dr_init->first) = dr_init->second;
-
   /* Advance data_reference's with the number of iterations of the previous
      loop and its prologue.  */
   vect_update_inits_of_drs (epilogue_vinfo, advance, PLUS_EXPR);
@@ -8551,7 +8544,7 @@  vect_transform_loop (loop_vec_info loop_vinfo)
   epilogue = vect_do_peeling (loop_vinfo, niters, nitersm1, &niters_vector,
 			      &step_vector, &niters_vector_mult_vf, th,
 			      check_profitability, niters_no_overflow,
-			      &advance, orig_drs_init);
+			      &advance);
 
   if (LOOP_VINFO_SCALAR_LOOP (loop_vinfo)
       && LOOP_VINFO_SCALAR_LOOP_SCALING (loop_vinfo).initialized_p ())
@@ -8810,7 +8803,7 @@  vect_transform_loop (loop_vec_info loop_vinfo)
 
   if (epilogue)
     {
-      update_epilogue_loop_vinfo (epilogue, advance, orig_drs_init);
+      update_epilogue_loop_vinfo (epilogue, advance);
 
       epilogue->simduid = loop->simduid;
       epilogue->force_vectorize = loop->force_vectorize;
diff --git a/gcc/tree-vect-stmts.c b/gcc/tree-vect-stmts.c
index 2cb6b1544ea1216640220f0b4808406f94f8ae49..150716d6d313b7e70b59c6de2335728e2e89408c 100644
--- a/gcc/tree-vect-stmts.c
+++ b/gcc/tree-vect-stmts.c
@@ -6630,7 +6630,7 @@  check_scan_store (stmt_vec_info stmt_info, tree vectype,
       || loop_vinfo == NULL
       || LOOP_VINFO_FULLY_MASKED_P (loop_vinfo)
       || STMT_VINFO_GROUPED_ACCESS (stmt_info)
-      || !integer_zerop (DR_OFFSET (dr_info->dr))
+      || !integer_zerop (get_dr_vinfo_offset (dr_info))
       || !integer_zerop (DR_INIT (dr_info->dr))
       || !(ref_type = reference_alias_ptr_type (DR_REF (dr_info->dr)))
       || !alias_sets_conflict_p (get_alias_set (vectype),
@@ -7763,6 +7763,7 @@  vectorizable_store (stmt_vec_info stmt_info, gimple_stmt_iterator *gsi,
       tree running_off;
       tree stride_base, stride_step, alias_off;
       tree vec_oprnd;
+      tree dr_offset;
       unsigned int g;
       /* Checked by get_load_store_type.  */
       unsigned int const_nunits = nunits.to_constant ();
@@ -7770,11 +7771,12 @@  vectorizable_store (stmt_vec_info stmt_info, gimple_stmt_iterator *gsi,
       gcc_assert (!LOOP_VINFO_FULLY_MASKED_P (loop_vinfo));
       gcc_assert (!nested_in_vect_loop_p (loop, stmt_info));
 
+      dr_offset = get_dr_vinfo_offset (first_dr_info);
       stride_base
 	= fold_build_pointer_plus
 	    (DR_BASE_ADDRESS (first_dr_info->dr),
 	     size_binop (PLUS_EXPR,
-			 convert_to_ptrofftype (DR_OFFSET (first_dr_info->dr)),
+			 convert_to_ptrofftype (dr_offset),
 			 convert_to_ptrofftype (DR_INIT (first_dr_info->dr))));
       stride_step = fold_convert (sizetype, DR_STEP (first_dr_info->dr));
 
@@ -8137,7 +8139,7 @@  vectorizable_store (stmt_vec_info stmt_info, gimple_stmt_iterator *gsi,
 	      && !loop_masks
 	      && TREE_CODE (DR_BASE_ADDRESS (first_dr_info->dr)) == ADDR_EXPR
 	      && VAR_P (TREE_OPERAND (DR_BASE_ADDRESS (first_dr_info->dr), 0))
-	      && integer_zerop (DR_OFFSET (first_dr_info->dr))
+	      && integer_zerop (get_dr_vinfo_offset (first_dr_info))
 	      && integer_zerop (DR_INIT (first_dr_info->dr))
 	      && alias_sets_conflict_p (get_alias_set (aggr_type),
 					get_alias_set (TREE_TYPE (ref_type))))
@@ -8831,6 +8833,7 @@  vectorizable_load (stmt_vec_info stmt_info, gimple_stmt_iterator *gsi,
       /* Checked by get_load_store_type.  */
       unsigned int const_nunits = nunits.to_constant ();
       unsigned HOST_WIDE_INT cst_offset = 0;
+      tree dr_offset;
 
       gcc_assert (!LOOP_VINFO_FULLY_MASKED_P (loop_vinfo));
       gcc_assert (!nested_in_vect_loop);
@@ -8861,11 +8864,12 @@  vectorizable_load (stmt_vec_info stmt_info, gimple_stmt_iterator *gsi,
 	  ref_type = reference_alias_ptr_type (DR_REF (dr_info->dr));
 	}
 
+      dr_offset = get_dr_vinfo_offset (first_dr_info);
       stride_base
 	= fold_build_pointer_plus
 	    (DR_BASE_ADDRESS (first_dr_info->dr),
 	     size_binop (PLUS_EXPR,
-			 convert_to_ptrofftype (DR_OFFSET (first_dr_info->dr)),
+			 convert_to_ptrofftype (dr_offset),
 			 convert_to_ptrofftype (DR_INIT (first_dr_info->dr))));
       stride_step = fold_convert (sizetype, DR_STEP (first_dr_info->dr));
 
@@ -9330,7 +9334,7 @@  vectorizable_load (stmt_vec_info stmt_info, gimple_stmt_iterator *gsi,
 	  if (simd_lane_access_p
 	      && TREE_CODE (DR_BASE_ADDRESS (first_dr_info->dr)) == ADDR_EXPR
 	      && VAR_P (TREE_OPERAND (DR_BASE_ADDRESS (first_dr_info->dr), 0))
-	      && integer_zerop (DR_OFFSET (first_dr_info->dr))
+	      && integer_zerop (get_dr_vinfo_offset (first_dr_info))
 	      && integer_zerop (DR_INIT (first_dr_info->dr))
 	      && alias_sets_conflict_p (get_alias_set (aggr_type),
 					get_alias_set (TREE_TYPE (ref_type)))
diff --git a/gcc/tree-vectorizer.h b/gcc/tree-vectorizer.h
index 51a13f1d20742d4b17811d2ab90900b730da5ea0..afbf4f4ce2af1bbb0b9a4907677ab1753959d072 100644
--- a/gcc/tree-vectorizer.h
+++ b/gcc/tree-vectorizer.h
@@ -910,6 +910,10 @@  public:
   /* If true the alignment of base_decl needs to be increased.  */
   bool base_misaligned;
   tree base_decl;
+
+  /* Stores current vectorized loop's offset.  To be added to the DR's
+     offset to calculate current offset of data reference.  */
+  tree offset;
 };
 
 typedef struct data_reference *dr_p;
@@ -1485,6 +1489,26 @@  vect_dr_behavior (dr_vec_info *dr_info)
     return &STMT_VINFO_DR_WRT_VEC_LOOP (stmt_info);
 }
 
+inline tree
+get_dr_vinfo_offset (dr_vec_info *dr_info, bool check_outer = false)
+{
+  innermost_loop_behavior *base;
+  if (check_outer)
+    base = vect_dr_behavior (dr_info);
+  else
+    base = &dr_info->dr->innermost;
+
+  tree offset = base->offset;
+
+  if (!dr_info->offset)
+    return offset;
+
+  offset = fold_convert (sizetype, offset);
+  return fold_build2 (PLUS_EXPR, TREE_TYPE (dr_info->offset), offset,
+		      dr_info->offset);
+}
+
+
 /* Return true if the vect cost model is unlimited.  */
 static inline bool
 unlimited_cost_model (loop_p loop)
@@ -1655,7 +1679,7 @@  class loop *slpeel_tree_duplicate_loop_to_edge_cfg (class loop *,
 class loop *vect_loop_versioning (loop_vec_info);
 extern class loop *vect_do_peeling (loop_vec_info, tree, tree,
 				    tree *, tree *, tree *, int, bool, bool,
-				    tree *, drs_init_vec &);
+				    tree *);
 extern void vect_prepare_for_masked_peels (loop_vec_info);
 extern dump_user_location_t find_loop_location (class loop *);
 extern bool vect_can_advance_ivs_p (loop_vec_info);