Uh oh!
There was an error while loading. Please reload this page.
JIT: Move internal reserved registers to a side table - #101647
Merged
Conversation
Contributor
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
MemberAuthor
TP regressions seem a little higher than expected (though the larger ones are MinOpts in collections that don't really have any MinOpts contexts). For Seems mostly inlining related. |
jakobbotsch
commented
Apr 27, 2024
MemberAuthor
For I'll see if we can do something. |
MemberAuthor
I got rid of an unnecessary hash map lookup in
runtime/src/coreclr/jit/lsraarmarch.cpp Line 193 in 55d2ada I'm inclined to just have the backend always use Edit: Avoided this internal register by using LR instead, which had large TP improvements for crossgen2 arm64 compilations. |
jakobbotsch
commented
Apr 28, 2024
MemberAuthor
The remaining one is libraries_tests_no_tiered_compilation.run.windows.arm64.Release.mch and looks to be the same as above, i.e. primarily because of some different inlining decisions: |
jakobbotsch
marked this pull request as ready for review
April 28, 2024 12:45
MemberAuthor
cc @dotnet/jit-contrib PTAL @kunalspathak This gets rid of For the TP hit in some of the other collections see my comments above. Most of the hit comes from different inlining decisions that we expect to be significantly altered by native PGO anyway, or MinOpts in collections with very few MinOpts contexts that thus aren't very representative. |
matouskozak pushed a commit
to matouskozak/runtime
that referenced
this pull request
Apr 30, 2024
This gets rid of `GenTree::gtRsvdRegs` by moving internal registers to a side table. We generally use internal registers very rarely, so making the lookup more costly seems worth the trade off (especially to make it easier to expand `regMaskTP` to 16 bytes). There was one exception where we used internal registers a lot, which was `GT_CALL` for R2R codegen on arm64/arm32. For those nodes we always allocate an internal register to load the target into (the target is obtained by loading the R2R indirection cell that is passed in an argument register). For arm64 it was simple to avoid this internal register: we can simply use LR always, since that register is going to be overwritten by the call anyway. This results in -2% TP for crossgen2 arm64 just from avoiding building this extra interval. This is also the cause of the asm diffs. For arm32 the same strategy doesn't work as well because loading into LR is a 4 byte instruction while loading into other registers is a 2 byte instruction. So for arm32 we still use an internal register and take the small throughput hit. This change reduces JIT memory usage by ~1.5%. The throughput cost (when discounting some spurious inlining decision changes) seems to be around 0.1%.
michaelgsharp pushed a commit
to michaelgsharp/runtime
that referenced
this pull request
May 9, 2024
This gets rid of `GenTree::gtRsvdRegs` by moving internal registers to a side table. We generally use internal registers very rarely, so making the lookup more costly seems worth the trade off (especially to make it easier to expand `regMaskTP` to 16 bytes). There was one exception where we used internal registers a lot, which was `GT_CALL` for R2R codegen on arm64/arm32. For those nodes we always allocate an internal register to load the target into (the target is obtained by loading the R2R indirection cell that is passed in an argument register). For arm64 it was simple to avoid this internal register: we can simply use LR always, since that register is going to be overwritten by the call anyway. This results in -2% TP for crossgen2 arm64 just from avoiding building this extra interval. This is also the cause of the asm diffs. For arm32 the same strategy doesn't work as well because loading into LR is a 4 byte instruction while loading into other registers is a 2 byte instruction. So for arm32 we still use an internal register and take the small throughput hit. This change reduces JIT memory usage by ~1.5%. The throughput cost (when discounting some spurious inlining decision changes) seems to be around 0.1%.
Ruihan-Yin pushed a commit
to Ruihan-Yin/runtime
that referenced
this pull request
May 30, 2024
This gets rid of `GenTree::gtRsvdRegs` by moving internal registers to a side table. We generally use internal registers very rarely, so making the lookup more costly seems worth the trade off (especially to make it easier to expand `regMaskTP` to 16 bytes). There was one exception where we used internal registers a lot, which was `GT_CALL` for R2R codegen on arm64/arm32. For those nodes we always allocate an internal register to load the target into (the target is obtained by loading the R2R indirection cell that is passed in an argument register). For arm64 it was simple to avoid this internal register: we can simply use LR always, since that register is going to be overwritten by the call anyway. This results in -2% TP for crossgen2 arm64 just from avoiding building this extra interval. This is also the cause of the asm diffs. For arm32 the same strategy doesn't work as well because loading into LR is a 4 byte instruction while loading into other registers is a 2 byte instruction. So for arm32 we still use an internal register and take the small throughput hit. This change reduces JIT memory usage by ~1.5%. The throughput cost (when discounting some spurious inlining decision changes) seems to be around 0.1%.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Paying a bit more to access these seems worth it when it leads to a 10% reduction in size of
GenTree. It's very rare for an IR node to have any internal registers allocated.For arm64 this is the distribution of number of internal registers allocated for every
GenTreeafter LSRA:Memory diff for arm64: https://www.diffchecker.com/fGt6Abyp/ (around 1.5% less memory used overall and makes it cheaper to expand to more than 64 registers in the future)
Contributes to #98258