From 3c764560c15a858dbe3423a030307abb5f0b4e0b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Mar 2026 23:17:08 +0000 Subject: [PATCH 1/2] Initial plan From 134bf148f82df6b08ed192174c977f5017b275c9 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Mar 2026 23:47:26 +0000 Subject: [PATCH 2/2] Fix JIT register move ordering in genConsumeBlockOp for CpBlkOp When the LSRA allocates the source address to the same register that the destination address needs (dstReg), the original code would incorrectly clobber the source address by copying the destination first: mov rdi, rax ; dst -> dstReg (CLOBBERS srcAddr already in rdi!) mov rsi, rdi ; src -> srcReg (copies destination, not source!) Fix by detecting this case and moving the source to srcReg first when srcAddr is currently in dstReg. Also update the existing comment which incorrectly claimed the LSRA always guarantees safe copy ordering, and add an assert to catch the unhandled swap case (srcAddr in dstReg AND dstAddr in srcReg simultaneously) which would produce incorrect code. Fixes: System.Diagnostics.Metrics.Tests.MetricsTests.PassingVariableTagsParametersTest under JitStress=2 + JitStressRegs={2,0x80} on Linux x64. Co-authored-by: mangod9 <61718172+mangod9@users.noreply.github.com> Agent-Logs-Url: https://github.com/dotnet/runtime/sessions/1b13221b-a0c1-4742-8445-5ada9974b0de --- src/coreclr/jit/codegenlinear.cpp | 36 +++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/codegenlinear.cpp b/src/coreclr/jit/codegenlinear.cpp index 2be7f9c272edad..31eef204e80bce 100644 --- a/src/coreclr/jit/codegenlinear.cpp +++ b/src/coreclr/jit/codegenlinear.cpp @@ -2084,9 +2084,9 @@ void CodeGen::genConsumeBlockOp(GenTreeBlk* blkNode, regNumber dstReg, regNumber // // Note that the register allocator ensures that the registers ON THE NODES will not interfere // with one another if consumed (i.e. reloaded or moved to their ASSIGNED reg) in execution order. - // Further, it ensures that they will not interfere with one another if they are then copied - // to the REQUIRED register (if a fixed register requirement) in execution order. This requires, - // then, that we first consume all the operands, then do any necessary moves. + // However, the copies to the REQUIRED registers (fixed register requirements) may interfere if + // the source address happens to be allocated to dstReg. We handle that ordering explicitly below. + // This requires, then, that we first consume all the operands, then do any necessary moves. GenTree* const dstAddr = blkNode->Addr(); @@ -2099,8 +2099,36 @@ void CodeGen::genConsumeBlockOp(GenTreeBlk* blkNode, regNumber dstReg, regNumber genConsumeBlockSrc(blkNode); // Next, perform any necessary moves. + // We must be careful about move ordering: if the source address is currently in dstReg, + // we must move source to srcReg first, before overwriting dstReg with the destination address. + // Otherwise, copying dst to dstReg would clobber the source address value. + bool srcMovedFirst = false; + if (srcReg != REG_NA && blkNode->OperIsCopyBlkOp()) + { + GenTree* src = blkNode->Data(); + if (src->OperIs(GT_IND)) + { + regNumber srcCurReg = src->AsOp()->gtOp1->GetRegNum(); + if ((srcCurReg != REG_NA) && (srcCurReg == dstReg)) + { + // Source is in the register that destination needs; move source first to avoid clobbering it. + // Note: a true swap (srcAddr in dstReg AND dstAddr in srcReg) would require a temporary + // register, but the register allocator should prevent this by ensuring at most one of the + // two operands ends up in the other's required register. + assert(dstAddr->GetRegNum() != srcReg); + genSetBlockSrc(blkNode, srcReg); + srcMovedFirst = true; + } + } + } + genCopyRegIfNeeded(dstAddr, dstReg); - genSetBlockSrc(blkNode, srcReg); + + if (!srcMovedFirst) + { + genSetBlockSrc(blkNode, srcReg); + } + genSetBlockSize(blkNode, sizeReg); }