-
Notifications
You must be signed in to change notification settings - Fork 5.3k
[RyuJit Wasm] don't home locals if none are referenced #123569
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
In such cases we won't have set up a stack pointer.
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 updates the WASM JIT codegen to avoid homing parameters when no locals or arguments require stack-based storage, i.e., when no stack pointer register has been set up.
Changes:
- Add a guard in
CodeGen::genHomeRegisterParamsthat checksGetStackPointerReg()and skips parameter homing when it isREG_NA. - Assert that the frame pointer is not used in this frameless case and emit a JITDUMP message for debugging.
|
@dotnet/jit-contrib PTAL Fixes codegen for methods with no (user) arguments, like public static int Zero() => 0; |
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
| { | ||
| JITDUMP("*************** In genHomeRegisterParams()\n"); | ||
|
|
||
| if (GetStackPointerReg() == REG_NA) |
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.
I think this is a situation where the RA is wrong; with implicitly referenced variables we have to assume there may be some references to them outside of IR. So something like below:
diff --git a/src/coreclr/jit/regallocwasm.cpp b/src/coreclr/jit/regallocwasm.cpp
index 48bcd978abd..8c001fa77e7 100644
--- a/src/coreclr/jit/regallocwasm.cpp
+++ b/src/coreclr/jit/regallocwasm.cpp
@@ -100,6 +100,10 @@ void WasmRegAlloc::IdentifyCandidates()
{
JITDUMP("RA candidate: V%02u", lclNum);
}
+ else if ((m_fpReg == REG_NA) && (varDsc->lvRefCnt() != 0))
+ {
+ m_fpReg = AllocateFreeRegister(TYP_I_IMPL);
+ }
}
}
@@ -147,11 +151,6 @@ void WasmRegAlloc::AllocateAndResolveNode(GenTree* node)
{
RewriteLocalStackStore(node->AsLclVarCommon());
}
-
- if (m_fpReg == REG_NA)
- {
- m_fpReg = AllocateFreeRegister(TYP_I_IMPL);
- }
}
}
else if (node->OperIs(GT_LCLHEAP))
diff --git a/src/coreclr/jit/registeropswasm.cpp b/src/coreclr/jit/registeropswasm.cpp
index 0386d477d1b..b8f96cc35a7 100644
--- a/src/coreclr/jit/registeropswasm.cpp
+++ b/src/coreclr/jit/registeropswasm.cpp
@@ -109,7 +109,10 @@ unsigned UnpackWasmReg(regNumber reg, WasmValueType* pType)
//
unsigned WasmRegToIndex(regNumber reg)
{
- return UnpackWasmReg(reg);
+ WasmValueType wasmType;
+ unsigned index = UnpackWasmReg(reg, &wasmType);
+ assert((WasmValueType::Invalid < wasmType) && (wasmType < WasmValueType::Count));
+ return index;
}
bool genIsValidReg(regNumber reg)
In such cases we won't have set up a stack pointer.