Hi Victor, > -----Original Message----- > From: Jakub Jelinek <[email protected]> > Sent: 18 December 2025 15:04 > To: Victor Do Nascimento <[email protected]> > Cc: [email protected]; Tamar Christina <[email protected]>; > [email protected]; [email protected] > Subject: Re: [PATCH] vect: Fix dominator update [PR123152] > > On Thu, Dec 18, 2025 at 02:54:14PM +0000, Victor Do Nascimento wrote: > > The `recompute_dominator' function used in the code fragment within > > this patch assumes correctness in the rest of the CFG. Consequently, > > it is wrong to rely upon it before the subsequent updates are made in > > the "Update dominators for multiple exits" loop in the function. > > > > Furthermore, if `loop_exit' == `scalar_exit', the "Update dominators for > > multiple exits" logic will already take care of updating the > > dominator for `scalar_exit->dest', such that the moved statement is > > unnecessary. > > Just formatting nits, will defer to Richi or Tamar for actual review. > > > gcc/ChangeLog: > > > > PR tree-optimization/123152 > > * tree-vect-loop-manip.cc > > (slpeel_tree_duplicate_loop_to_edge_cfg): Correct order of > > dominator update > > * tree-vect-loop-manip.cc (slpeel_tree_duplicate_loop_to_edge_cfg): > Correct order of dominator update. > fits in nicely into 76 columns, no need to split line before that. And > don't forget the dot at the end of change description. > > > @@ -2024,6 +2019,11 @@ slpeel_tree_duplicate_loop_to_edge_cfg (class > loop *loop, edge loop_exit, > > } > > } > > } > > + > > + if (create_main_e && (loop_exit != scalar_exit)) > > Why the ()s around the comparison? > > > + set_immediate_dominator (CDI_DOMINATORS, scalar_exit->dest, > > + recompute_dominator (CDI_DOMINATORS, > > + scalar_exit->dest)); > > } >
It took me a while to understand what this was doing. If I understood correctly the issue is that when we do prologue peeling with uncounted loops we insert a dummy exit and that exit replaces scalar_exit in new_exit. And because of this the edge redirection of new_exit no longer updates the dominators. Most of the time this is fine, however when there is a third copy of the loop due to if-conversion then the dominator of scalar_exit needs to be updated because it's going to be dominated by a common block between the peeled loop and the if-converted loop because copy_bbs will leave the re-use the exit block's destination for the new copy by default. So patch is OK with the nits from Jakub, but It would be good to add a small comment here explaining the above because it's not an entirely obvious thing. Thanks, Tamar > Jakub
