JIT: Preference SDSU intervals away from killed registers#125219
Merged
jakobbotsch merged 10 commits intodotnet:mainfrom Mar 16, 2026
Merged
JIT: Preference SDSU intervals away from killed registers#125219jakobbotsch merged 10 commits intodotnet:mainfrom
jakobbotsch merged 10 commits intodotnet:mainfrom
Conversation
A def-use conflict happens for SDSU intervals when the def and use share no common register. For these cases we run through a set of potential ways to resolve the conflict that make use of the contract between LSRA and codegen. For example, when the definition requires a single fixed register, it is still valid for LSRA to assign a different register. This means that codegen can use the original fixed register, but that it must insert a copy into the register assigned by LSRA. In the worst case we cannot use any of the approaches to solve the conflict and then we will fall back to spilling/copying as necessary. It appears that several of the cases were overly conservative. Particularly case 1 and 2, as described by the function header, were never reachable; we assign `defRegConflict` and `useRegConflict` based on whether there is a conflict in register constraints, but that is always true when we get into this function. It appears the original intention of these variables were whether there is a conflict with a _different_ use of the registers we could otherwise have resolved the original conflict with. This PR restores the meaning to that.
github-actions
bot
added
the
area-CodeGen-coreclr
Contributor
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Contributor
There was a problem hiding this comment.
Pull request overview
Revives prior LSRA heuristics so SDSU (single-def/single-use) temp intervals are preferentially allocated away from registers killed by the consuming node, addressing regressions observed after updated def/use conflict handling.
Changes:
- Reworks
resolveConflictingDefAndUseconflict tracking so cases like #5 become reachable again and adjusts fixed-reg handling in case #3. - Refactors kill-driven preference updates into
updateIntervalPreferencesForKilland applies it to both live locals and in-flight (defList) non-local intervals. - Adds the new helper declaration to
lsra.h.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/coreclr/jit/lsrabuild.cpp |
Updates SDSU def/use conflict resolution logic; refactors and extends kill-based register preference updates; adds helper implementation. |
src/coreclr/jit/lsra.h |
Declares the new updateIntervalPreferencesForKill helper on LinearScan. |
Comments suppressed due to low confidence (1)
src/coreclr/jit/lsrabuild.cpp:347
- In case #4 we update
useRefPosition->registerAssignmentbut keepuseRefPosition->isFixedRegReftrue.RegisterSelection::select()asserts thatisFixedRegRefimplies a single-bitregisterAssignment, and keeping the fixed flag also means we’re effectively changing the fixed register requirement todefRegAssignment. With the new meaning ofdefRegConflict, this branch can now execute whendefRegAssignmentis multi-reg (or just different from the original fixed reg), which can violate the fixed-reg invariant and/or cause incorrect codegen assumptions. Consider either requiringdefRefPosition->isFixedRegRef/isSingleRegister(defRegAssignment)for case #4, or clearinguseRefPosition->isFixedRegRef(similar to case #3/#5) when you change its candidates away from the fixed register.
if ((useReg != REG_NA) && !defRegConflict && canChangeUseAssignment)
{
// This is case #4.
INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_DEFUSE_CASE4, interval));
useRefPosition->registerAssignment = defRegAssignment;
return;
Member
Author
This was referenced Mar 10, 2026
EgorBo
approved these changes
Mar 13, 2026
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 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.
This revives #81242 on top of #125214. I noticed some regressions with #125214 because we were missing this logic (specifically case 5 when resolving DU conflicts). Also, this may have some TP implications that I want to look at separately from the other change.
Example: