-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Move RBM_SAVED_LOCALLOC_SP reservation from compCompile to setFrameType in LSRA #121745
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
base: main
Are you sure you want to change the base?
Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
Co-authored-by: jakobbotsch <7887810+jakobbotsch@users.noreply.github.com>
| // We reserve REG_SAVED_LOCALLOC_SP to store SP on entry for stack unwinding | ||
| compiler->codeGen->regSet.rsMaskResvd |= RBM_SAVED_LOCALLOC_SP; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a JITDUMP like the one above about the fact that we are reserving this register?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Can you add a JITDUMP like the one above about the fact that we are reserving this register?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in 8da758e
Co-authored-by: jakobbotsch <7887810+jakobbotsch@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR relocates ARM-specific register reservation code from the compCompile function to the setFrameType function in LSRA. The change moves the logic that reserves RBM_SAVED_LOCALLOC_SP (R9 on ARM) for stack unwinding when localloc is used, placing it closer to related register reservation code.
Key changes:
- Removes the
#ifdef TARGET_ARMblock fromcompCompileincompiler.cpp - Adds the same logic to
setFrameTypeinlsra.cpp, positioned after theRBM_OPT_RSVDreservation code - Includes a new JITDUMP statement for debugging visibility when the register is reserved
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/coreclr/jit/compiler.cpp | Removed ARM-specific register reservation block from compCompile function |
| src/coreclr/jit/lsra.cpp | Added ARM-specific register reservation to setFrameType with JITDUMP logging |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Move the code that adds
RBM_SAVED_LOCALLOC_SPtorsMaskResvdfromcompCompileintosetFrameTypein LSRAPlan
compiler.cpplines 4953-4959 (insidecompCompile)lsra.cppsetFrameType()function after line 2718compiler.cpp(lines 4953-4959)lsra.cppinsetFrameType()after existingrsMaskResvdsettingChanges Made
#ifdef TARGET_ARMblock that setsRBM_SAVED_LOCALLOC_SPinrsMaskResvdfrom thecompCompilefunctionsetFrameType()after the existing code that setsrsMaskResvdfor large frame offsetsREG_SAVED_LOCALLOC_SPis reservedBuild Status
✅ CoreCLR build succeeded with changes
✅ CoreCLR build succeeded with JITDUMP addition
✅ No CodeQL security issues detected
Security Summary
No security vulnerabilities were introduced or detected in this change. The code is a straightforward refactoring that moves existing logic to a more appropriate location without changing its behavior.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.