JIT: Remove side effect detection quirk in loop hoisting #118463
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves side effect detection during loop hoisting by making the traversal more flow-sensitive. Previously, the JIT was overly conservative, assuming every block after the loop header could execute after the first global side effect. The new approach propagates side effect state through the control flow paths of the loop, enabling more precise hoisting decisions.
Key changes:
- Replaced global
m_beforeSideEffectflag with per-block side effect tracking usingm_sideEffectBlocksBitVec - Modified side effect detection to propagate through control flow rather than assuming worst-case after first block
- Updated comments to reflect the new flow-sensitive approach
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
This reverts commit 217d6e7.
|
cc @dotnet/jit-contrib, @AndyAyersMS PTAL. I previously tried a more sophisticated approach for propagating side effect state more precisely through conditional paths, but this seemed to overwhelm CSE with candidates, leading to about a 50/50 split of large PerfScore improvements and regressions. To make that approach work, I'll probably have to further quirk the JIT to only hoist newly valid candidates if they're sufficiently expensive. For now, this change yields small but largely positive diffs. |
|
ping @AndyAyersMS |
…net#118463)" This reverts commit e0b14e9. That change is not correct as it removes the safe guard that prevents hoisting out of conditionally executed blocks.
Addresses some of the regressions in #116486 and #116754. Hoisting's traversal of the loop body was previously not sensitive to execution flow, so it was overly conservative in its side effect detection, operating under the assumption that every block after the loop header could execute after the first global side effect. Now that hoisting visits every loop block in RPO, we can remove this assumption.