On Fri, Sep 04, 2026 at 10:20:37AM -0600, Jeff Law wrote:


On 9/3/26 7:13 PM, Robert Guthrie wrote:
It's likely harder than a typical "simple hack of the week" as those often end up being target patterns or match.pd patterns -- so limited in how many APIs you need to understand, much less RTL or gimple IL parsing, etc.  But there's folks here that can help.


That is true: it has been a little harder than I expected!
Since I am modifying cleanup_cfg, and it is called so many times,
I am getting a lot of weird interactions leading to infinite loops or
other errors. Or getting different behavior when it is called in CFGRTL vs. CFGLAYOUT
mode (I spent last evening trying to grok the difference).
Maybe this is just something to tag onto the end of bbro rather than adding it
to a core function instead?
But it fits fairly naturally in cfg cleanup and thinking more about it, it should work irrespective of whether we're in cfg layout mode or not if we do the cfg manipulations correctly.

The whole point of splitting out the two modes is to allow earlier passes to ignore the low level mechanics of dealing with fallthru edges and the like.

Conceptually a block ending in a return can have its return removed and the cfg altered so that it has a fallthru edge to a block that has a return and nothing else.   This is true before and after cfg layout mode.


Ok, I have had a couple more evenings to dig at this.
I feel like something like the diff below is quite simple and works?
Let me explain how I got here for some context:

* First, I started unaware there was a difference between RTL and Layout mode.
  Whatever I was doing was not handling Fallthru correctly.
* Next, I tried to change how I handled Fallthrus, and it was breaking Layout 
mode
  (mainly because the EDGE_FALLTHRU flag means different things)
* I ignored Layout mode for a bit and tried to just get RTL mode working.
  I was doing what you originally suggested: delete the first block and 
redirect its successors
  to the second one. I was having a lot of trouble with Fallthrus, such as:
  - the infinite loop I had was: I was accidentally creating fwding blocks, 
which were just an
    unconditional jump-to-ret. Another transformation in `try_optimize_cfg` 
turned that back into a
    ret, which then hit my transform again...
  - in some cases, the fallthru predecessor of the first ret-only block was 
also jumping
    to the second ret-only, so whatever code I had ended up with duplicate edges
    (e.g, if I had BB ret-only blocks 4 and 5, and 3->4 and 3->5, then deleting 4 and 
redirecting 3->4 to 3->5
    created a dupe)

I suppose these things are in principle able to be handled, but rather than 
whack-a-mole instead I just deleted the second block,
redirecting its precessors to the first, which got rid of all the Fallthru 
problems (since there necessarily are not fallthrus).
Then, I think it also works in Layout mode as well, and it is ultimately pretty 
simple.
This bootstrapped correctly (on x86, I ordered a RISC-V dev board but haven't 
gotten it yet, though it solves my original problem too when I build cross).
I need to learn how to add good test cases before submitting as a formal patch, 
I am just dropping the diff
below:

diff --git a/gcc/cfgcleanup.cc b/gcc/cfgcleanup.cc
index 1d9ec908dab..d74bfcf10e2 100644
--- a/gcc/cfgcleanup.cc
+++ b/gcc/cfgcleanup.cc
@@ -2831,14 +2831,39 @@ try_optimize_cfg (int mode)
                      redirect_edge_succ (single_succ_edge (b),
                                          EXIT_BLOCK_PTR_FOR_FN (cfun));
                      single_succ_edge (b)->flags &= ~EDGE_CROSSING;
                      changed_here = true;
                    }
                }
+ /* If we have two consecutive return-only basic blocks,
+                delete the second one, redirecting all of its predecessors
+                to the first one.  */
+             if (!(mode & CLEANUP_NO_INSN_DEL)
+                 && bb_is_just_return (b, &ret, &use)
+                 && bb_is_just_return (b->prev_bb, &ret, &use))
+               {
+                  basic_block pred = b->prev_bb;
+                  edge e;
+                  for (edge_iterator ei = ei_start (b->preds);
+                       (e = ei_safe_edge (ei));)
+                    redirect_edge_and_branch_force (e, pred);
+
+                  if (dump_file)
+                    fprintf (dump_file,
+                      "deleted unnecessary ret-only block %d by "
+                      "re-targeting to %d\n",
+                      b->index, pred->index);
+
+                  delete_basic_block (b);
+                  changed = true;
+                  b = pred;
+                  continue;
+               }
+
              /* Try to change a conditional branch to a return to the
                 respective conditional return.  */
              if (EDGE_COUNT (b->succs) == 2
                  && any_condjump_p (BB_END (b))
                  && bb_is_just_return (BRANCH_EDGE (b)->dest, &ret, &use))

One curiosity, I found a string of 3 ret's in symtab.o, which this only reduced 
to two, not one... I haven't debugged that one yet though.





BTW, I haven't gotten a response from gcc-bugzilla-account-request (not sure if that is typical,
I'm sure there is a lot of spam sadly) but I have not forgotten about it.
They were swamped with yet another DDOS earlier this week.  They may just be getting a little sleep.

Jeff

Got it, no worries.

Thanks again
Robert

Reply via email to