JIT: maintain pred lists during loop unrolling

[AndyAyersMS]

We now update pred lists during loop unrolling, rather than recomputing them from scratch.

There are several parts to the fix: first, optRedirectBlock now has a new ability to add pred references for the flow from a newly cloned block, be it either to a remapped successor or a non-remapped successor. Along with this we no longer copy over the block ref count in CloneBlockState. These changes allow us to create the right pred links and ref counts in the interior of a cloned subgraph.

Second, we now scrub block references from the original loop body blocks instead of just setting their ref counts to zero.

Finally, we fix up references for exterior flow into and out of the unroll complex.

Addresses one of the cases mentioned in #49030.

[ghost]

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

Issue Details

---

We now update pred lists during loop unrolling, rather than recomputing them from scratch.

There are several parts to the fix: first, optRedirectBlock' now has a new ability to add pred references for the flow from a newly cloned block, be it either to a remapped successor or a non-remapped successor. Along with this we no longer copy over the block ref count in CloneBlockState`. These changes allow us to create the right pred links and ref counts in the interior of a cloned subgraph.

Second, we now scrub block references from the original loop body blocks instead of just setting their ref counts to zero.

Finally, we fix up references for exterior flow into and out of the unroll complex.

Addresses one of the cases mentioned in #49030.

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

[AndyAyersMS]

@BruceForstall PTAL
cc @dotnet/jit-contrib

Also contains an unrelated jit dump fix for SSA.

This was annoyingly hard to get right; the main culprits being

• the proper way to "gut" the existing loop body -- ended up needing two passes
• handling the case where we're actually removing the loop

Also will note some ideas for future cleanup to the unroller. These may deserve a new issue or notes on existing issues:

• We are seemingly too aggressive unmarking alignment; if we unroll a loop with nested aligned loops it seems like those cloned nested loops should continue to be aligned
• As noted in the code the profile update doesn't make sense. Will try and fix this subsequently
• Seems like the "zero" case should be handled elsewhere/separately?
• Seems like the "one" case we shouldn't need to clone? And in general perhaps we should be making N-1 copies of the IR instead of making N and then trashing the original. I get that we may not get constant prop right away but it may not matter.

[AndyAyersMS]

Also I will work on fixing the other places we trash pred lists, so have reassigned #49030.

[AndyAyersMS]

Hmm, I am seeing some SPMI diffs locally, which is a bit surprising -- let me verify I have a good baseline jit before I start digging in deeper.

[AndyAyersMS]

Hmm, I am seeing some SPMI diffs locally, which is a bit surprising -- let me verify I have a good baseline jit before I start digging in deeper.

Looks like I didn't rebuild my diff branch after rebasing onto main. No diffs (as expected) once it was rebuilt.

[AndyAyersMS]

Diffs—as expected, no code diffs.

Looks like TP is a wash, extra cost maintaining preds evidently balances savings from not rebuilding preds. I thought it might be a bit cheaper to maintain.

[AndyAyersMS]

@BruceForstall FYI I have the other phases that drop preds list fixed (building on top of these changes), but will keep them as a follow-up PR.

[BruceForstall]

LGTM

[BruceForstall]

nit

Suggested change

	// We fall into to this unroll iteration from the bottom block (first iteration)
	// We fall into this unroll iteration from the bottom block (first iteration)

?

[AndyAyersMS]

I'll fix this in the next PR...