JIT: update bbNums before inverting loops

[AndyAyersMS]

We use the bbNums to decide how to manipulate "in loop" edges when inverting loops, and this was sometimes misleading as the bbNums did not reflect block order.

Per the code this is not a correctness issue, but needless redirections can confuse loop recognition.

Fixes #80809.

Diffs

[ghost]

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

Issue Details

---

We use the bbNum to decide how to manipulate "in loop" edges when inverting loops, and this was sometimes misleading as the bbNums did not reflect block order.

Per the code this is not a correctness issue, but needless redirections can confuse loop recognition.

Fixes #80809.

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

[AndyAyersMS]

@BruceForstall PTAL
cc @dotnet/jit-contrib

Expect some minor diffs.

[BruceForstall]

You decided to explicitly renumber blocks even under SMALL_CODE -- is that to provide a guarantee that the blocks are always properly renumbered after this phase no matter what? (as long as loop inversion is not disabled by config)

[AndyAyersMS]

You decided to explicitly renumber blocks even under SMALL_CODE -- is that to provide a guarantee that the blocks are always properly renumbered after this phase no matter what? (as long as loop inversion is not disabled by config)

Wasn't quite sure what the exit contract should be. Right now it's a bit of a mess:

• if we don't optimize then the phase doesn't run (so no renumbering)
• if we optimize but have small code we renumber and on exit those numbers are still valid
• if we invert we add new blocks so the renumbering is no longer valid

Happy to reconsider this, I manly wanted to fix behavior so my follow-on changes for pred lists are no diff, and it appears downstream somewhere we are evidently careful/consistent about renumbering.

[BruceForstall]

Seems like it's better to renumber (and/or assert renumbering is not needed) before a phase starts that requires it, rather than (sometimes) renumbering to leave the numbers "canonical" after a phase.

[AndyAyersMS]

Seems like it's better to renumber (and/or assert renumbering is not needed) before a phase starts that requires it, rather than (sometimes) renumbering to leave the numbers "canonical" after a phase.

So you think I should leave this as is? Or make changes?

[edit] Since you approved I'll assume "leaving as is " is acceptable.