JIT: Generalize flow edge redirection logic

[amanasifkhalid]

Replace fgRedirect*Edge helpers with an implementation agnostic to the source block's jump kind.

I was motivated to do this consolidation while looking at replacing a few flow optimizations with block compaction. For example, fgOptimizeSwitchBranches shouldn't be necessary when we can easily compact the switch's successors. However, block compaction currently skips blocks with switch predecessors to avoid invalidating the switch descriptor map. If we add support for redirecting switch successor edges, we can update their flow without needing to modify the map most of the time (and I've found from my local work that when we do need to update the map, it's quite easy to do).

[dotnet-policy-service]

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

[Copilot]

Pull Request Overview

This PR consolidates the JIT’s flow-edge redirection helpers into a single, implementation-agnostic fgRedirectEdge method and updates all call sites accordingly.

• Replaces fgRedirectTargetEdge, fgRedirectTrueEdge, and fgRedirectFalseEdge with fgRedirectEdge at every call site.
• Adds GetTargetEdgeRef(), GetTrueEdgeRef(), and GetFalseEdgeRef() to BasicBlock and updates the Compiler interface.
• Adjusts switch-case lowering to manually reset duplicate counts instead of using the old helpers.

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
rangecheckcloning.cpp	Swapped fgRedirectTargetEdge for fgRedirectEdge.
optimizer.cpp	Updated conditional and loop-redirection calls.
optimizebools.cpp	Replaced boolean-optimization redirection.
objectalloc.cpp	Changed object-allocation clone redirection.
morph.cpp	Unified Qmark-statement flow updates.
lower.cpp	Refactored switch-case successor count adjustments.
jiteh.cpp	Swapped EH-region edge redirections.
indirectcalltransformer.cpp	Rewrote cold-path edge redirection.
importer.cpp	Updated impImportLeave* redirections.
helperexpansion.cpp	Unified runtime-lookup redirections.
async.cpp	Simplified resumption-switch redirection.
flowgraph.cpp	Replaced target-edge helper and cleaned up unreachable case.
fgopt.cpp	Streamlined post-import cleanup and branch optimizations.
fginline.cpp	Updated inline insertion redirection.
fgflow.cpp	Removed specialized redirection methods; added generic one.
fgehopt.cpp	Replaced finally-optimization edge redirections.
fgbasic.cpp	Adjusted jump-target replacement logic.
compiler.h	Updated Compiler API to expose fgRedirectEdge.
block.h	Added methods to obtain edge references on BasicBlock.

Comments suppressed due to low confidence (2)

src/coreclr/jit/fgflow.cpp:370

• [nitpick] The documentation comment still references the outdated behavior for a generic redirect; consider updating this comment to reflect that it now handles true, false, and unconditional edges uniformly and remove references to parameters that no longer exist.

// fgRedirectEdge: Sets the given edge's target block to 'newTarget',

src/coreclr/jit/rangecheckcloning.cpp:283

• It would be helpful to add or extend unit tests that verify fgRedirectEdge works correctly for a simple range-check cloning scenario, ensuring edge redirection preserves expected control flow and weights.

    comp->fgRedirectEdge(prevBb->GetTargetEdgeRef(), lowerBndBb);

[Copilot]

[nitpick] Consider adding a comment explaining why the successor edge duplicate counts are adjusted manually here instead of using the existing helpers (fgRemoveAllRefPreds/fgAddRefPred) to improve readability and maintainability.

Suggested change

	//
	//
	// Note: The successor edge duplicate counts are adjusted manually here instead of using
	// the existing helpers (`fgRemoveAllRefPreds`/`fgAddRefPred`) because this operation
	// is part of a tightly coupled optimization that directly modifies edge counts and
	// references. Using the helpers in this context would introduce unnecessary overhead
	// and complexity, as they are designed for more general-purpose scenarios.

[amanasifkhalid]

@dotnet/jit-contrib PTAL. No asmdiffs. Aside from a MinOpts outlier in coreclr_tests, the TP diffs are small. The generalized implementation has an extra branch compared to fgRedirectTargetEdge (i.e. the single-successor case), but this seems to make a tangible difference infrequently enough that I don't think it's worth templating this branch away.

[jakobbotsch]

Most other "ref" getters do not have "Get" prefix, e.g. NodeRef, ReturnValueRef, ArrRef. Should we follow the same convention here?

[amanasifkhalid]

The choice is between following that convention, or the naming scheme of the other BasicBlock getters (GetTargetEdge, etc.) -- which would you prefer?

[amanasifkhalid]

@jakobbotsch just following up on this

[jakobbotsch]

I guess the argument for not having the "Get" prefix is that you can both get and set the field through the reference. CallArg has both GetNode() and NodeRef() for example. I think it would make sense to follow the same convention here.

[amanasifkhalid]

Good point, I'll open a follow-up to switch the naming.

[amanasifkhalid]

ping @dotnet/jit-contrib