| Message ID | mpttv7agsid.fsf@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | Improve vector alias checks for WAR and WAW dependencies | expand |
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));
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));