JIT: update bbNums before inverting loops - #80827
Conversation
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 dotnet#80809.
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch, @kunalspathak Issue DetailsWe 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.
|
|
@BruceForstall PTAL Expect some minor diffs. |
BruceForstall
left a comment
There was a problem hiding this comment.
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:
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. |
|
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. |
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 dotnet#80809.
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