On Mon, Nov 11, 2019 at 7:47 PM Richard Sandiford
<richard.sandif...@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.sandif...@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));
