On Thu, 9 Nov 2023, Tamar Christina wrote:
> > guard_bb = LOOP_VINFO_IV_EXIT (loop_vinfo)->dest;
> > edge epilog_e = LOOP_VINFO_EPILOGUE_IV_EXIT (loop_vinfo);
> > - guard_to = split_edge (epilog_e);
> > + guard_to = epilog_e->dest;
> > guard_e = slpeel_add_loop_guard (guard_bb, guard_cond, guard_to,
> > skip_vector ? anchor : guard_bb,
> > prob_epilog.invert (),
> > @@ -3443,8 +3229,30 @@ vect_do_peeling (loop_vec_info loop_vinfo, tree
> > niters, tree nitersm1,
> > if (vect_epilogues)
> > epilogue_vinfo->skip_this_loop_edge = guard_e;
> > edge main_iv = LOOP_VINFO_IV_EXIT (loop_vinfo);
> > - slpeel_update_phi_nodes_for_guard2 (loop, epilog, main_iv,
> > guard_e,
> > - epilog_e);
> > + gphi_iterator gsi2 = gsi_start_phis (main_iv->dest);
> > + for (gphi_iterator gsi = gsi_start_phis (guard_to);
> > + !gsi_end_p (gsi); gsi_next (&gsi))
> > + {
> > + /* We are expecting all of the PHIs we have on epilog_e
> > + to be also on the main loop exit. But sometimes
> > + a stray virtual definition can appear at epilog_e
> > + which we can then take as the same on all exits,
> > + we've removed the LC SSA PHI on the main exit before
> > + so we wouldn't need to create a loop PHI for it. */
> > + if (virtual_operand_p (gimple_phi_result (*gsi))
> > + && (gsi_end_p (gsi2)
> > + || !virtual_operand_p (gimple_phi_result (*gsi2))))
> > + add_phi_arg (*gsi,
> > + gimple_phi_arg_def_from_edge (*gsi, epilog_e),
> > + guard_e, UNKNOWN_LOCATION);
> > + else
> > + {
> > + add_phi_arg (*gsi, gimple_phi_result (*gsi2), guard_e,
> > + UNKNOWN_LOCATION);
> > + gsi_next (&gsi2);
> > + }
> > + }
> > +
>
> I've been having some trouble incorporating this change into the early break
> work.
> My understanding is that here you've removed the lookup that find_guard did
> and are assuming that the order between the PHI nodes between loop->exit
> and epilog->exit are the same - sporadic virtual operands.
>
> But the loop->exit for early break has to materialize all PHI nodes from the
> main
> loop into the epilog loop since we need them to restart the scalar loop
> iteration.
>
> This means that the number of PHI nodes between the first loop and the second
> Loop are not the same, so we end up mis-linking phi nodes. i.e. consider
> this loop
>
> https://gist.github.com/Mistuke/65d476b18f991772fdec159a09b81869
I don't see any multi-exits here? I think you need exactly the same
PHIs you need for the branch to the epilogue, no?
If you can point me to a testcase that fails on your branch I can
try to have a look.
> which now goes wrong (throw that in a dotgraph viewer).
>
> I'm struggling to figure out how to handle this without doing a lookup.
>
> Any advice?
>
> Thanks,
> Tamar
>
>
> > /* Only need to handle basic block before epilog loop if it's not
> > the guard_bb, which is the case when skip_vector is true. */
> > if (guard_bb != bb_before_epilog)
> > --
> > 2.35.3
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
