On Tue, Jul 13, 2021 at 10:52 AM Jakub Jelinek <ja...@redhat.com> wrote: > > On Tue, Jul 13, 2021 at 09:56:41AM +0200, Richard Biener wrote: > > The comment above says > > > > /* During cfg pass make sure to put orphaned labels > > into the right OMP region. */ > > > > and the full guard is > > > > if ((unsigned) bb->index < bb_to_omp_idx.length () > > && ((unsigned) new_bb->index >= bb_to_omp_idx.length () > > || (bb_to_omp_idx[bb->index] > > != bb_to_omp_idx[new_bb->index]))) > > > > The <em>right</em> OMP region suggests something wrt correctness > > (though the original choice of bb->prev_bb doesn't put the initial new_bb > > in an semantic relation to the original block). > > The reason for the OMP code I've added there is to put orphaned labels into > the right OpenMP region, because if they are moved to arbitrary unrelated > locations, when outlining the OMP regions (e.g. parallel, task, or target), > we want the orphaned labels to be in the right function as opposed to some > unrelated one, at least if the orphaned label is referenced in the code. > Because referencing say from offloaded code a label that got moved to the > host function or vice versa simply can't work and similarly causes ICEs > even with parallel/task etc. regions. > > But I'm not sure I agree with the intention of the patch, yes, orphaned > labels even without OpenMP are moved to some other place, but the current > code typically keeps it close to where it used to live.
typically, yes, but bb->prev_bb doesn't really have such meaning. OTOH it's hard to do better from delete_basic_block, callers might do a better job but there's currently no way to communicate down sth like an entry or exit to a removed CFG region where these things could be moved to. Maybe the GIMPLE CFG hooks could queue to be re-issued stmts somewhere and callers be responsible for re-emitting them. > Moving them to > entry will be always worse user experience if people e.g. print the &&label > addresses etc. > > Jakub >
