JIT: Move internal reserved registers to a side table #101647
Merged
Conversation
This file contains 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
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
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. |
|
For I'll see if we can do something. |
|
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. |
|
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: |
|
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%.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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