On Wed, Aug 26, 2026 at 9:17 AM Richard Biener
<[email protected]> wrote:
>
> On Tue, Aug 25, 2026 at 7:48 PM Aldy Hernandez <[email protected]> wrote:
> >
> > The backward threader can resolve a computed goto's target along a
> > path, but the thread is dropped at copy time when it is the target of
> > another abnormal edge:
> >
> >   Registering jump thread: (2, 4) incoming edge;  (4, 5) nocopy;
> >   Cannot copy the blocks in the path:   Cancelling jump thread:
> >
> > The problem is that can_copy_bbs_p refuses any region with an
> > intra-region abnormal edge by design because it would end up
> > redirecting abnormal edges which is a no-no.
> >
> > Work around this by copying the last block by hand if we know we'll
> > end up stripping the abnormal flag when we resolve the path.  This
> > allows us to thread paths that end in an abnormal edge, though
> > abnormal edges mid-path are still disallowed.  Though that's not a
> > regression because AFAICT, DOM won't do that either.
> >
> > BTW, I explored various ways of attacking this, and ultimately landed
> > on the cheapest way to handle this by touching the least amount of
> > things, cause GCC parts tend to be sticky-- and I don't want to be
> > left holding the bag ;-).
> >
> > Pardon the copious comments and diagrams; I had a hard time wrapping
> > my head around this.
> >
> > OK?
>
> Huh, so what's the situation?  The last block has a control statement we
> can statically evaluate but also an abnormal outgoing edge?  And the
> result is a copy of the block but only with the static determined edge?

Ah, computed goto.  I think we simply want to allow redirecting of
abnormal edges, at least on GIMPLE I do not see what is technically
in the way here.  That we reject this might be a defensive measure to
prevent accidential redirections?  As with copying we're happily
creating new abnormal edges, redirecting those back into the copied
region looks reasonable.

Honza, didn't you have plans to make this work?

Aldy, does the forward threader handle the testcase or what's the reason
to try to fix this?

Richard.

> >
> >         PR tree-optimization/126906
> >
> > gcc/ChangeLog:
> >
> >         * tree-ssa-threadupdate.cc
> >         (back_jt_path_registry::duplicate_thread_path): Copy the last
> >         block by hand, keeping its outgoing edges on the original
> >         destinations; check the two copy phases separately.  Assert
> >         that the last-block redirect fixup no longer triggers.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.dg/tree-ssa/ssa-thread-25.c: New test.
> > ---
> >  gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-25.c | 30 ++++++++
> >  gcc/tree-ssa-threadupdate.cc                  | 77 +++++++++++++++----
> >  2 files changed, 94 insertions(+), 13 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-25.c
> >
> > diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-25.c 
> > b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-25.c
> > new file mode 100644
> > index 00000000000..526e96bd2c8
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-thread-25.c
> > @@ -0,0 +1,30 @@
> > +/* PR tree-optimization/126906 */
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 --param=dom-jump-threading=0 
> > -fdump-tree-ethread-details" } */
> > +
> > +/* The computed goto lives in the same block as the address-taken
> > +   label lab2, so the goto's abnormal edge to lab2 points back into
> > +   the block itself.  Threading the resolved paths must copy that
> > +   block, which the generic copier used to refuse.  */
> > +
> > +void *sink;
> > +
> > +void
> > +foo (int b)
> > +{
> > +  void *x;
> > +  sink = &&lab2;
> > +  if (b)
> > +    x = &&lab;
> > +  else
> > +    x = &&done;
> > +lab2:
> > +  goto *x;
> > +lab:
> > +  sink = 0;
> > +done:
> > +  return;
> > +}
> > +
> > +/* { dg-final { scan-tree-dump-times "Registering jump thread" 2 "ethread" 
> > } } */
> > +/* { dg-final { scan-tree-dump-not "Cancelling" "ethread" } } */
> > diff --git a/gcc/tree-ssa-threadupdate.cc b/gcc/tree-ssa-threadupdate.cc
> > index 54d2c419423..106d906548c 100644
> > --- a/gcc/tree-ssa-threadupdate.cc
> > +++ b/gcc/tree-ssa-threadupdate.cc
> > @@ -2395,11 +2395,22 @@ back_jt_path_registry::duplicate_thread_path (edge 
> > entry,
> >  {
> >    unsigned i;
> >    class loop *loop = entry->dest->loop_father;
> > -  edge exit_copy;
> >    edge redirected;
> >    profile_count curr_count;
> >
> > -  if (!can_copy_bbs_p (region, n_region))
> > +  gcc_checking_assert (n_region > 0);
> > +  basic_block last_bb = region[n_region - 1];
> > +
> > +  /* We're going to duplicate the last block by hand to avoid 
> > can_copy_bbs_p's
> > +     restriction of refusing to copy abnormal successor edges, so look at 
> > the
> > +     last block in isolation...  */
> > +  if (!can_duplicate_block_p (last_bb))
> > +    {
> > +      *failure_reason = "Cannot duplicate the last block in the path";
> > +      return false;
> > +    }
> > +  /* ...then verify the rest of the path is copyable.  */
> > +  if (!can_copy_bbs_p (region, n_region - 1))
> >      {
> >        *failure_reason = "Cannot copy the blocks in the path";
> >        return false;
> > @@ -2431,9 +2442,52 @@ back_jt_path_registry::duplicate_thread_path (edge 
> > entry,
> >
> >    set_loop_copy (loop, loop);
> >
> > +  /* Copy all but the last block with copy_bbs, then copy the last block 
> > (B3)
> > +     by hand.  For example, take the path B1 -> B2 -> B3 below, where B3 
> > ends
> > +     in a computed goto that the thread resolved to the exit edge B3 -ab-> 
> > lab,
> > +     and where one of the goto's other abnormal edges points back into the
> > +     region:
> > +
> > +       B1 -> B2 -> B3 -ab-> lab
> > +              ^    |
> > +              +-ab-+
> > +
> > +     copy_bbs redirects every copied edge that targets a copied block
> > +     into the copy.  With the above path, it would have to produce:
> > +
> > +       B1' -> B2' -> B3' -ab-> lab
> > +               ^     |
> > +               +-ab--+
> > +
> > +     redirecting the copied abnormal edge to B2'.  That cannot be done, 
> > because
> > +     the computed goto target is unknown at compile time.  This is why
> > +     can_copy_bbs_p refuses such regions.
> > +
> > +     But since we know the only possible path out of B1'->B2'->B3' is 
> > "lab", we
> > +     can clear the abnormal flag, and ultimately remove the intra-path 
> > abnormal
> > +     edge since we know it won't happen.
> > +
> > +     We will end up with:
> > +
> > +       B1' -> B2' -> B3' -> lab
> > +  */
> >    basic_block *region_copy = XNEWVEC (basic_block, n_region);
> > -  copy_bbs (region, n_region, region_copy, &exit, 1, &exit_copy, loop,
> > -           split_edge_bb_loc (entry), false);
> > +  basic_block after = split_edge_bb_loc (entry);
> > +  if (n_region > 1)
> > +    {
> > +      copy_bbs (region, n_region - 1, region_copy, NULL, 0, NULL, loop,
> > +               after, false);
> > +      after = region_copy[n_region - 2];
> > +    }
> > +  basic_block last_bb_copy = duplicate_block (last_bb, NULL, after);
> > +  region_copy[n_region - 1] = last_bb_copy;
> > +  if (n_region > 1)
> > +    {
> > +      /* The edge into the original last block now enters its copy.  */
> > +      edge sp = find_edge (after, last_bb);
> > +      gcc_assert (sp && !(sp->flags & EDGE_ABNORMAL));
> > +      redirect_edge_and_branch_force (sp, last_bb_copy);
> > +    }
> >
> >    /* Fix up: copy_bbs redirects all edges pointing to copied blocks.  The
> >       following code ensures that all the edges exiting the jump-thread 
> > path are
> > @@ -2482,17 +2536,14 @@ back_jt_path_registry::duplicate_thread_path (edge 
> > entry,
> >           continue;
> >         }
> >
> > -      /* Special case the last block on the path: make sure that it does 
> > not
> > -        jump back on the copied path, including back to itself.  */
> > +      /* There's nothing to redirect here, since the last block on the 
> > path was
> > +        copied by hand and its edges point at the original destinations.  A
> > +        follow-up patch will remove this assert.  */
> >        if (i + 1 == n_region)
> >         {
> > -         FOR_EACH_EDGE (e, ei, bb->succs)
> > -           if (bb_in_bbs (e->dest, region_copy, n_region))
> > -             {
> > -               basic_block orig = get_bb_original (e->dest);
> > -               if (orig)
> > -                 redirect_edge_and_branch_force (e, orig);
> > -             }
> > +         if (flag_checking)
> > +           FOR_EACH_EDGE (e, ei, bb->succs)
> > +             gcc_assert (!bb_in_bbs (e->dest, region_copy, n_region));
> >           continue;
> >         }
> >
> > --
> > 2.47.3
> >

Reply via email to