[RISCV] redundant return-only BB not eliminated

[Jeff Law]

On 9/6/26 1:06 PM, Robert Guthrie wrote:
> 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)
Sometimes its better to ask for help as you work through these things.  
Ideally with before/after dumps and the patch you're working with.  GCC 
is fairly complex and often even very experienced developers need to 
throw things under the debugger to really understand what's happening.

>
> 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).
Note that RISC-V development boards are still quite slow.  To give you a 
sense, my 2020-ish servers can do a bootstrap & regression test in about 
90 minutes.  On my K3 board, it's 9 hours.


> I need to learn how to add good test cases before submitting as a 
> formal patch, I am just dropping the diff
> below:
Lots of examples for testcases ;-)
>
> 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))
So in general you don't want to be looking at prev_bb/next_bb.  Yes, 
blocks are kept in a chain, but there's really no meaning to the order 
of the chain until after block layout.  What really matters are the 
pred/succ lists.

So roughly

/* Find predecessor of exit block that is just a return and nothing 
else.  */
FOR_EACH_EDGE (e, ei, EXIT_BLOCK_PTR_FOR_FN (cfun)->preds)
   {
     if (bb_is_just_return (e->pred))
       {
         target_bb = e->pred;
         break;
   }

if (target_bb)
   {
     FOR_EACH_EDGE (e, ei, EXIT_BLOCK_PTR_FOR_FN (cfun)->preds)
       {
         if (e->pred == target_bb)
           continue;

         /* If the block unconditionally reaches EXIT_BLOCK, then 
redirect it to TARGET_BB.  */
         if (single_succ_p (e->pred))
           {
              /* CFG Manipulations */
           }
       }
   }

As I'm scribbling this code out, I do recall that we had this kind of 
transformation.   I see the remnants of one in reorg.cc, but that's not 
something I'd really suggest copying.  That doesn't really work the 
CFG.  I seem to recall it was in jump.cc, but that's been largely gutted 
and subsumed by cfgcleanup, but I don't see anything useful in cfgcleanup.

Jeff