[2/8] Delay swapping data refs in prune_runtime_alias_test_list
diff mbox series

Message ID mpttv7agsid.fsf@arm.com
State New
Headers show
Series
  • Improve vector alias checks for WAR and WAW dependencies
Related show

Commit Message

Richard Sandiford Nov. 11, 2019, 6:47 p.m. UTC
prune_runtime_alias_test_list swapped dr_as between two dr_with_seg_len
pairs before finally deciding whether to merge them.  Bailing out later
would therefore leave the pairs in an incorrect state.

IMO a better fix would be to split this out into a subroutine that
produces a temporary dr_with_seg_len on success, rather than changing
an existing one in-place.  It would then be easy to merge both the dr_as
and dr_bs if we wanted to, rather than requiring one of them to be equal.
But here I tried to do something that could be backported if necessary.


2019-11-11  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* tree-data-ref.c (prune_runtime_alias_test_list): Delay
	swapping the dr_as based on init values until we've decided
	whether to merge them.

Comments

Richard Biener Nov. 15, 2019, 11:01 a.m. UTC | #1
On Mon, Nov 11, 2019 at 7:47 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> prune_runtime_alias_test_list swapped dr_as between two dr_with_seg_len
> pairs before finally deciding whether to merge them.  Bailing out later
> would therefore leave the pairs in an incorrect state.
>
> IMO a better fix would be to split this out into a subroutine that
> produces a temporary dr_with_seg_len on success, rather than changing
> an existing one in-place.  It would then be easy to merge both the dr_as
> and dr_bs if we wanted to, rather than requiring one of them to be equal.
> But here I tried to do something that could be backported if necessary.

OK.

>
> 2019-11-11  Richard Sandiford  <richard.sandiford@arm.com>
>
> gcc/
>         * tree-data-ref.c (prune_runtime_alias_test_list): Delay
>         swapping the dr_as based on init values until we've decided
>         whether to merge them.
>
> Index: gcc/tree-data-ref.c
> ===================================================================
> --- gcc/tree-data-ref.c 2019-11-11 18:30:43.203244558 +0000
> +++ gcc/tree-data-ref.c 2019-11-11 18:30:47.199216669 +0000
> @@ -1556,13 +1556,6 @@ prune_runtime_alias_test_list (vec<dr_wi
>           if (!ordered_p (init_a1, init_a2))
>             continue;
>
> -         /* Make sure dr_a1 starts left of dr_a2.  */
> -         if (maybe_gt (init_a1, init_a2))
> -           {
> -             std::swap (*dr_a1, *dr_a2);
> -             std::swap (init_a1, init_a2);
> -           }
> -
>           /* Work out what the segment length would be if we did combine
>              DR_A1 and DR_A2:
>
> @@ -1579,7 +1572,10 @@ prune_runtime_alias_test_list (vec<dr_wi
>
>              The lengths both have sizetype, so the sign is taken from
>              the step instead.  */
> -         if (!operand_equal_p (dr_a1->seg_len, dr_a2->seg_len, 0))
> +         poly_uint64 new_seg_len = 0;
> +         bool new_seg_len_p = !operand_equal_p (dr_a1->seg_len,
> +                                                dr_a2->seg_len, 0);
> +         if (new_seg_len_p)
>             {
>               poly_uint64 seg_len_a1, seg_len_a2;
>               if (!poly_int_tree_p (dr_a1->seg_len, &seg_len_a1)
> @@ -1597,14 +1593,24 @@ prune_runtime_alias_test_list (vec<dr_wi
>               int sign_a = tree_int_cst_sgn (indicator_a);
>               int sign_b = tree_int_cst_sgn (indicator_b);
>
> -             poly_uint64 new_seg_len;
>               if (sign_a <= 0 && sign_b <= 0)
>                 new_seg_len = lower_bound (seg_len_a1, seg_len_a2);
>               else if (sign_a >= 0 && sign_b >= 0)
>                 new_seg_len = upper_bound (seg_len_a1, seg_len_a2);
>               else
>                 continue;
> +           }
> +         /* At this point we're committed to merging the refs.  */
>
> +         /* Make sure dr_a1 starts left of dr_a2.  */
> +         if (maybe_gt (init_a1, init_a2))
> +           {
> +             std::swap (*dr_a1, *dr_a2);
> +             std::swap (init_a1, init_a2);
> +           }
> +
> +         if (new_seg_len_p)
> +           {
>               dr_a1->seg_len = build_int_cst (TREE_TYPE (dr_a1->seg_len),
>                                               new_seg_len);
>               dr_a1->align = MIN (dr_a1->align, known_alignment (new_seg_len));

Patch
diff mbox series

Index: gcc/tree-data-ref.c
===================================================================
--- gcc/tree-data-ref.c	2019-11-11 18:30:43.203244558 +0000
+++ gcc/tree-data-ref.c	2019-11-11 18:30:47.199216669 +0000
@@ -1556,13 +1556,6 @@  prune_runtime_alias_test_list (vec<dr_wi
 	  if (!ordered_p (init_a1, init_a2))
 	    continue;
 
-	  /* Make sure dr_a1 starts left of dr_a2.  */
-	  if (maybe_gt (init_a1, init_a2))
-	    {
-	      std::swap (*dr_a1, *dr_a2);
-	      std::swap (init_a1, init_a2);
-	    }
-
 	  /* Work out what the segment length would be if we did combine
 	     DR_A1 and DR_A2:
 
@@ -1579,7 +1572,10 @@  prune_runtime_alias_test_list (vec<dr_wi
 
 	     The lengths both have sizetype, so the sign is taken from
 	     the step instead.  */
-	  if (!operand_equal_p (dr_a1->seg_len, dr_a2->seg_len, 0))
+	  poly_uint64 new_seg_len = 0;
+	  bool new_seg_len_p = !operand_equal_p (dr_a1->seg_len,
+						 dr_a2->seg_len, 0);
+	  if (new_seg_len_p)
 	    {
 	      poly_uint64 seg_len_a1, seg_len_a2;
 	      if (!poly_int_tree_p (dr_a1->seg_len, &seg_len_a1)
@@ -1597,14 +1593,24 @@  prune_runtime_alias_test_list (vec<dr_wi
 	      int sign_a = tree_int_cst_sgn (indicator_a);
 	      int sign_b = tree_int_cst_sgn (indicator_b);
 
-	      poly_uint64 new_seg_len;
 	      if (sign_a <= 0 && sign_b <= 0)
 		new_seg_len = lower_bound (seg_len_a1, seg_len_a2);
 	      else if (sign_a >= 0 && sign_b >= 0)
 		new_seg_len = upper_bound (seg_len_a1, seg_len_a2);
 	      else
 		continue;
+	    }
+	  /* At this point we're committed to merging the refs.  */
 
+	  /* Make sure dr_a1 starts left of dr_a2.  */
+	  if (maybe_gt (init_a1, init_a2))
+	    {
+	      std::swap (*dr_a1, *dr_a2);
+	      std::swap (init_a1, init_a2);
+	    }
+
+	  if (new_seg_len_p)
+	    {
 	      dr_a1->seg_len = build_int_cst (TREE_TYPE (dr_a1->seg_len),
 					      new_seg_len);
 	      dr_a1->align = MIN (dr_a1->align, known_alignment (new_seg_len));