From 545f04734b3ef5865499e9b206a980d862b85e66 Mon Sep 17 00:00:00 2001
From: Jakob Botsch Nielsen <jakob.botsch.nielsen@gmail.com>
Date: Mon, 20 Apr 2026 23:11:45 +0200
Subject: [PATCH] JIT: Avoid changing def registers that may assume an implicit
 kill

LSRA build has places where it assumes that `BuildDef(tree, SRBM_REG)`
means that `SRBM_REG` is going to be spilled if something is in it,
without constructing any explicit kill. This assumption is incompatible
with the logic in `resolveConflictingDefAndUse` that changes the def
register on fixed reg definitions -- the required spilling will happen
only if there ends up being a def into that register.

We could fix all the places that assume this and add explicit kills, but
instead this change just avoids changing the def assignment away from
the register when it detects there is an active interval assigned to the
fixed def register.
---
 src/coreclr/jit/lsrabuild.cpp | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/src/coreclr/jit/lsrabuild.cpp b/src/coreclr/jit/lsrabuild.cpp
index 6ba3fbd4ae84f4..e6a83bbb7de8de 100644
--- a/src/coreclr/jit/lsrabuild.cpp
+++ b/src/coreclr/jit/lsrabuild.cpp
@@ -261,15 +261,30 @@ void LinearScan::resolveConflictingDefAndUse(Interval* interval, RefPosition* de
 
     INDEBUG(dumpLsraAllocationEvent(LSRA_EVENT_DEFUSE_CONFLICT));
 
-    if (defRefPosition->treeNode->IsMultiRegNode())
-    {
-        // If the defRefPosition is multireg then we cannot change any of its
-        // register assignments. That could cause us to require a parallel
-        // assignment during codegen that could have cycles in it. For example,
-        // x64 DivRem always defines into RAX and RDX, so it would be a problem
-        // to change the register assignments to RDX and RAX respectively.
-    }
-    else
+    // If the defRefPosition is multireg then we cannot change any of its
+    // register assignments. That could cause us to require a parallel
+    // assignment during codegen that could have cycles in it. For example,
+    // x64 DivRem always defines into RAX and RDX, so it would be a problem
+    // to change the register assignments to RDX and RAX respectively.
+    bool canChangeDef = !defRefPosition->treeNode->IsMultiRegNode();
+
+    // Avoid changing the def reg away from its assignment if that register is
+    // currently busy. The reason is that we have a number of places in LSRA
+    // that assume that BuildDef(tree, SRBM_REG) means that SRBM_REG will be
+    // spilled if necessary, so they do not bother to kill SRBM_REG explicitly.
+    // However, that spilling happens only if we actually assign the def to
+    // SRBM_REG.
+    // A slightly better way would be to also unassign the fixed reg always,
+    // but this is mostly a stress-only case because the presence of the fixed
+    // reg on the def should make most intervals prefer not to be allocated to
+    // that register.
+    if (canChangeDef && defRefPosition->isFixedRegRef)
+    {
+        RegRecord* defRegRecord = getRegisterRecord(defRefPosition->assignedReg());
+        canChangeDef = (defRegRecord->assignedInterval == nullptr) || !defRegRecord->assignedInterval->isActive;
+    }
+
+    if (canChangeDef)
     {
         if (useRefPosition->isFixedRegRef)
         {