Skip to content

JIT: avoid store forward stall for struct params in GS frames #127487

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

Draft
AndyAyersMS wants to merge 1 commit into
base: main
Choose a base branch
from
+32 −8
Draft

JIT: avoid store forward stall for struct params in GS frames #127487

Show file tree
Hide file tree
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
8 changes: 7 additions & 1 deletion src/coreclr/jit/codegenxarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3434,13 +3434,19 @@ void CodeGen::genCodeForCpBlkUnroll(GenTreeBlk* node)
unsigned srcAddrIndexScale = 1;
int srcOffset = 0;
GenTree* src = node->Data();
bool srcIsRegArg = false;

assert(src->isContained());

if (src->OperIs(GT_LCL_VAR, GT_LCL_FLD))
{
srcLclNum = src->AsLclVarCommon()->GetLclNum();
srcOffset = src->AsLclVarCommon()->GetLclOffs();

if (src->OperIs(GT_LCL_VAR))
{
srcIsRegArg = m_compiler->lvaGetDesc(srcLclNum)->lvIsMultiRegArg;
}
Comment on lines 3436 to +3449
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

The new boolean srcIsRegArg is derived from lvIsMultiRegArg (multi-register struct arg), not from lvIsRegArg/a generic register-argument property. Renaming it to something like srcIsMultiRegArg would make the intent clearer and avoid confusion with the existing lvIsRegArg flag on locals.

Copilot uses AI. Check for mistakes.
}
else
{
Expand Down Expand Up @@ -3486,7 +3492,7 @@ void CodeGen::genCodeForCpBlkUnroll(GenTreeBlk* node)
// Get the largest SIMD register available if the size is large enough
unsigned regSize = m_compiler->roundDownSIMDSize(size);

if ((size >= regSize) && (regSize > 0))
if ((size >= regSize) && (regSize > 0) && !srcIsRegArg)
{
regNumber tempReg = internalRegisters.GetSingle(node, RBM_ALLFLOAT);

Expand Down
19 changes: 14 additions & 5 deletions src/coreclr/jit/lowerxarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -467,18 +467,27 @@ void Lowering::LowerBlockStore(GenTreeBlk* blkNode)
assert(src->OperIs(GT_IND, GT_LCL_VAR, GT_LCL_FLD));
src->SetContained();

bool isMultiRegArgCopy = false;

if (src->OperIs(GT_LCL_VAR))
{
// TODO-1stClassStructs: for now we can't work with STORE_BLOCK source in register.
const unsigned srcLclNum = src->AsLclVar()->GetLclNum();
m_compiler->lvaSetVarDoNotEnregister(srcLclNum DEBUGARG(DoNotEnregisterReason::StoreBlkSrc));
isMultiRegArgCopy = m_compiler->lvaGetDesc(srcLclNum)->lvIsMultiRegArg;
}

ClassLayout* layout = blkNode->GetLayout();
bool doCpObj = layout->HasGCPtr();
bool isNotHeap = blkNode->IsAddressNotOnHeap(m_compiler);
bool canUseSimd = !doCpObj || isNotHeap;

if (isMultiRegArgCopy)
{
canUseSimd = false;
}

ClassLayout* layout = blkNode->GetLayout();
bool doCpObj = layout->HasGCPtr();
bool isNotHeap = blkNode->IsAddressNotOnHeap(m_compiler);
bool canUseSimd = !doCpObj || isNotHeap;
unsigned copyBlockUnrollLimit = m_compiler->getUnrollThreshold(Compiler::UnrollKind::Memcpy, canUseSimd);
unsigned copyBlockUnrollLimit = m_compiler->getUnrollThreshold(Compiler::UnrollKind::Memcpy, canUseSimd);

#ifndef JIT32_GCENCODER
if (doCpObj && (size <= copyBlockUnrollLimit))
Expand Down
13 changes: 11 additions & 2 deletions src/coreclr/jit/lsraxarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1495,7 +1495,15 @@ int LinearScan::BuildBlockStore(GenTreeBlk* blkNode)
unsigned regSize = m_compiler->roundDownSIMDSize(size);
unsigned remainder = size;

if ((size >= regSize) && (regSize > 0))
bool srcIsRegArg = false;

if (src->OperIs(GT_LCL_VAR))
{
unsigned srcLclNum = src->AsLclVar()->GetLclNum();
srcIsRegArg = m_compiler->lvaGetDesc(srcLclNum)->lvIsMultiRegArg;
}
Comment on lines +1498 to +1504
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

The new flag name srcIsRegArg is misleading here: it is set based on lvIsMultiRegArg (multi-register struct arg), not lvIsRegArg or a general "reg arg" condition. Consider renaming to something like srcIsMultiRegArg (or similar) to avoid confusion with the existing lvIsRegArg field and to better document why SIMD is being disabled.

Copilot uses AI. Check for mistakes.

if ((size >= regSize) && (regSize > 0) && !srcIsRegArg)
{
// We need a float temporary if we're doing SIMD operations

Expand All @@ -1505,7 +1513,8 @@ int LinearScan::BuildBlockStore(GenTreeBlk* blkNode)
remainder %= regSize;
}

if ((remainder > 0) && ((regSize == 0) || (isPow2(remainder) && (remainder <= REGSIZE_BYTES))))
if (srcIsRegArg ||
((remainder > 0) && ((regSize == 0) || (isPow2(remainder) && (remainder <= REGSIZE_BYTES)))))
{
// We need an int temporary if we're not doing SIMD operations
// or if are but the remainder is a power of 2 and less than the
Expand Down
Loading