Skip to content

Reduce TP for targets with more than 64 Registers Part 2 : Make operations on fixedRegs less expensive #113713

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 27, 2025

Conversation

DeepakRajendrakumaran
Copy link
Contributor

@DeepakRajendrakumaran DeepakRajendrakumaran commented Mar 19, 2025

Rationale for this change

The reason why updateNextFixedRef is expensive is due to us repeatedly working on fixedRegs which is a regMaskTP. This change splits fixedRegs into 2 SingleTypeRegSet variables fixedRegsLow and fixedRegsHigh. This necessitates some changes to how we call the impacted methods. Implementing updateNextFixedRef as a templated method allows us to have 2 separate versions of this method without adding duplicate methods

Changes included here

  • fixedRegs is split into 2 separate SingleTypeRegSet on targets with more than 64 registers
  • A templated updateNextFixedRef implementation that works on the newly created SingleTypeRegSet
  • A updateNextFixedRefDispatch which calls the required updateNextFixedRef based on register number.

Impact of this change

This change should have minimal to no impact on targets with less than 64 registers. Local testing done with additional eGPR enabled(to simulate more than 64 registers) show the following results

Overall (-0.35% to -0.12%)
Collection PDIFF
aspnet.run.windows.x64.checked.mch -0.27%
benchmarks.run.windows.x64.checked.mch -0.19%
benchmarks.run_pgo.windows.x64.checked.mch -0.22%
benchmarks.run_pgo_optrepeat.windows.x64.checked.mch -0.19%
coreclr_tests.run.windows.x64.checked.mch -0.34%
libraries.crossgen2.windows.x64.checked.mch -0.21%
libraries.pmi.windows.x64.checked.mch -0.22%
libraries_tests.run.windows.x64.Release.mch -0.35%
libraries_tests_no_tiered_compilation.run.windows.x64.Release.mch -0.33%
realworld.run.windows.x64.checked.mch -0.23%
smoke_tests.nativeaot.windows.x64.checked.mch -0.12%
MinOpts (-1.09% to -0.31%)
Collection PDIFF
aspnet.run.windows.x64.checked.mch -0.71%
benchmarks.run.windows.x64.checked.mch -0.52%
benchmarks.run_pgo.windows.x64.checked.mch -0.67%
benchmarks.run_pgo_optrepeat.windows.x64.checked.mch -0.52%
coreclr_tests.run.windows.x64.checked.mch -0.39%
libraries.crossgen2.windows.x64.checked.mch -0.31%
libraries.pmi.windows.x64.checked.mch -1.09%
libraries_tests.run.windows.x64.Release.mch -0.81%
libraries_tests_no_tiered_compilation.run.windows.x64.Release.mch -0.79%
realworld.run.windows.x64.checked.mch -0.38%
smoke_tests.nativeaot.windows.x64.checked.mch -0.37%
FullOpts (-0.32% to -0.12%)
Collection PDIFF
aspnet.run.windows.x64.checked.mch -0.21%
benchmarks.run.windows.x64.checked.mch -0.19%
benchmarks.run_pgo.windows.x64.checked.mch -0.16%
benchmarks.run_pgo_optrepeat.windows.x64.checked.mch -0.19%
coreclr_tests.run.windows.x64.checked.mch -0.30%
libraries.crossgen2.windows.x64.checked.mch -0.21%
libraries.pmi.windows.x64.checked.mch -0.22%
libraries_tests.run.windows.x64.Release.mch -0.21%
libraries_tests_no_tiered_compilation.run.windows.x64.Release.mch -0.32%
realworld.run.windows.x64.checked.mch -0.23%
smoke_tests.nativeaot.windows.x64.checked.mch -0.12%

Will help with reducing regression in 'procesKills()` - #112962

@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 19, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Mar 19, 2025
Copy link
Contributor

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

@DeepakRajendrakumaran DeepakRajendrakumaran force-pushed the killreg branch 2 times, most recently from df9d60f to a91ef0b Compare March 20, 2025 22:01
@DeepakRajendrakumaran DeepakRajendrakumaran force-pushed the killreg branch 2 times, most recently from 56dc88c to 134eea5 Compare March 24, 2025 19:04
@DeepakRajendrakumaran DeepakRajendrakumaran changed the title Draft : Reduce TP by handling liveRegs separately Draft : Make operations on fixedRegs less expensive Mar 24, 2025
@DeepakRajendrakumaran DeepakRajendrakumaran force-pushed the killreg branch 2 times, most recently from 030b684 to d2b16f9 Compare March 24, 2025 23:25
@DeepakRajendrakumaran DeepakRajendrakumaran force-pushed the killreg branch 2 times, most recently from 97ea8dd to 477aa53 Compare March 25, 2025 23:50
@DeepakRajendrakumaran DeepakRajendrakumaran changed the title Draft : Make operations on fixedRegs less expensive Reduce TP for targets with more than 64 Registers Part 2 : Make operations on fixedRegs less expensive Mar 26, 2025
@DeepakRajendrakumaran DeepakRajendrakumaran marked this pull request as ready for review March 26, 2025 20:05
@DeepakRajendrakumaran
Copy link
Contributor Author

@BruceForstall @kunalspathak This PR is ready for review.

@BruceForstall
Copy link
Contributor

odd superpmi-replay failure. trying again.

/azp run runtime-coreclr superpmi-replay

@BruceForstall
Copy link
Contributor

/azp run runtime-coreclr superpmi-replay

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Nice TP wins!

@kunalspathak
Copy link
Member

/ba-g failures are timeout

@kunalspathak kunalspathak merged commit 0ff4bf2 into dotnet:main Mar 27, 2025
114 of 116 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Apr 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants