Merge return true/false blocks

[EgorBo]

I plan to unconditionally expand all return <condition> blocks to JTRUE, merge tails (this PR) and then fold to return <condition>; back if tails were not found.

Diffs

[ghost]

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

---

CI experiment, some nice diffs locally

Author:	EgorBo
Assignees:	EgorBo
Labels:	area-CodeGen-coreclr
Milestone:	-

[EgorBo]

@AndyAyersMS @jakobbotsch @dotnet/jit-contrib PTAL, the current shape is what @jakobbotsch suggested in #94387 (comment) (basically, his branch), my initial one was focusing on return <bool> and turned out to be too conservative.

I've limitted it to single-statement RETURN blocks, but it seems like it makes sense to enable it for all, but that will require some careful analysis of TP/Size regressions (up to +0.4% TP impact) + needs a small fix in morph for explicit tail calls (I or Jakob will try to land it separately).
There 3 source of regressions:

1. Loop clonning (esp in aspnet collection), example is https://www.diffchecker.com/doR6p4zH/ (checked via JitDump that it's indeed loop clonning)
2. In some cases we no longer emit branchless cmove, etc due to a different layout of blocks
3. In some cases we light up a frame in x64 leaf functions, my understanding that it's because of that BB count heuristic

but overall it's a clear size win: https://dev.azure.com/dnceng-public/public/_build/results?buildId=463538&view=results

I also expect to see some improvements when I expand return <compare> into jtrue separately

[jakobbotsch]

Is a statement with a null node possible? Overall, do we actually find any NOPs in the return blocks?

[EgorBo]

I wasn't sure in this, but if I remove the nullcheck it won't fail on CI so I presume it's not possible? I wish we could have a centralized "IR reference" where we could check what's legal and what's not in IR

[EgorBo]

Is a statement with a null node possible? Overall, do we actually find any NOPs in the return blocks?

I'll check the diffs

[SingleAccretion]

It is not possible.

[EgorBo]

Reverted these changes, there were no additional diffs from that

[jakobbotsch]

It would be nice to convert these to functions instead of massive lambdas... but that's a no-diff refactoring to be done separately.

[jakobbotsch]

I think this should be reverted (and fixed in the future PR), or we should check all the tailcall flags.

[EgorBo]

Reverted

[AndyAyersMS]

I've limitted it to single-statement RETURN blocks, but it seems like it makes sense to enable it for all, but that will require some careful analysis of TP/Size regressions (up to +0.4% TP impact) + needs a small fix in morph for explicit tail calls (I or Jakob will try to land it separately).

I'm surprised checkin more statements has such a TP impact; it should be fairly self-limiting as there can't be that many return blocks. Unless it's enabling a lot more merging?

[AndyAyersMS]

I don't have anything to add other that what's already been noted.

Cool that this was relatively refactoring simple; I recall working on this a while back but got bogged down for some reason.

[EgorBo]

I'm surprised checkin more statements has such a TP impact;

it's possible that the TP regressions in that case come from extra loop clonning etc, I'll check separately, this PR will help to isolate those changes