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. Moving them to
entry will be always worse user experience if people e.g. print the &&label
addresses etc.
Jakub
