Skip to content

[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.

Sign up for GitHub

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

Open
AndyAyersMS wants to merge 1 commit into
base: main
Choose a base branch
from
+10 −0
Open

[RyuJit Wasm] don't home locals if none are referenced #123569

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/coreclr/jit/codegenwasm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,16 @@ void CodeGen::genHomeRegisterParams(regNumber initReg, bool* initRegStillZeroed)
{
JITDUMP("*************** In genHomeRegisterParams()\n");

if (GetStackPointerReg() == REG_NA)
Copy link
Contributor

@SingleAccretion SingleAccretion Jan 24, 2026

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)

Copy link
Member Author

@AndyAyersMS AndyAyersMS Jan 25, 2026

Choose a reason for hiding this comment

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

That works but the codegen is a bit wonky.

0003a9 func[9] <wasm_test_Program__Zero>:
 0003aa: 20 00                      | local.get 0
 0003ac: 41 08                      | i32.const 8
 0003ae: 6b                         | i32.sub
 0003af: 21 00                      | local.set 0
 0003b1: 20 00                      | local.get 0
 0003b3: 20 00                      | local.get 0
 0003b5: 36 00 04                   | i32.store 0 4
 0003b8: 20 00                      | local.get 0
 0003ba: 20 01                      | local.get 1
 0003bc: 36 00 00                   | i32.store 0 0
 0003bf: 41 00                      | i32.const 0
 0003c1: 0b                         | end

Copy link
Contributor

@SingleAccretion SingleAccretion Jan 25, 2026

Choose a reason for hiding this comment

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

the codegen is a bit wonky.

It is how it is. We don't try to optimize unused locals in debug codegen.

{
// We didn't see any local or argument references, so there's nothing to spill.
// TODO-WASM: debug codegen would likely spill all the user args anyways.
//
JITDUMP(" No local references -- skipping parameter homing\n");
assert(!isFramePointerUsed());
return;
}

auto spillParam = [this](unsigned lclNum, unsigned offset, unsigned paramLclNum, const ABIPassingSegment& segment) {
assert(segment.IsPassedInRegister());

Expand Down
Loading