Skip to content

Don't use ZR as target in LSE atomics #105854

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

Merged
merged 4 commits into from
Aug 5, 2024
Merged

Don't use ZR as target in LSE atomics #105854

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
87c95da
Don't use ZR as target in LSE atomics
EgorBo Aug 2, 2024
26c30c6
Update codegenarm64.cpp
EgorBo Aug 2, 2024
bc83340
Update lsraarm64.cpp
EgorBo Aug 2, 2024
b0473ba
Update lsraarm64.cpp
EgorBo Aug 2, 2024
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
13 changes: 7 additions & 6 deletions src/coreclr/jit/codegenarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3878,19 +3878,21 @@ void CodeGen::genLockedInstructions(GenTreeOp* treeNode)
{
assert(!data->isContainedIntOrIImmed());

// These instructions change semantics when targetReg is ZR (the memory ordering becomes weaker).
// See atomicBarrierDroppedOnZero in LLVM
assert((targetReg != REG_NA) && (targetReg != REG_ZR));

switch (treeNode->gtOper)
{
case GT_XORR:
GetEmitter()->emitIns_R_R_R(INS_ldsetal, dataSize, dataReg, (targetReg == REG_NA) ? REG_ZR : targetReg,
addrReg);
GetEmitter()->emitIns_R_R_R(INS_ldsetal, dataSize, dataReg, targetReg, addrReg);
break;
case GT_XAND:
{
// Grab a temp reg to perform `MVN` for dataReg first.
regNumber tempReg = internalRegisters.GetSingle(treeNode);
GetEmitter()->emitIns_R_R(INS_mvn, dataSize, tempReg, dataReg);
GetEmitter()->emitIns_R_R_R(INS_ldclral, dataSize, tempReg, (targetReg == REG_NA) ? REG_ZR : targetReg,
addrReg);
GetEmitter()->emitIns_R_R_R(INS_ldclral, dataSize, tempReg, targetReg, addrReg);
break;
}
case GT_XCHG:
Expand All @@ -3908,8 +3910,7 @@ void CodeGen::genLockedInstructions(GenTreeOp* treeNode)
break;
}
case GT_XADD:
GetEmitter()->emitIns_R_R_R(INS_ldaddal, dataSize, dataReg, (targetReg == REG_NA) ? REG_ZR : targetReg,
addrReg);
GetEmitter()->emitIns_R_R_R(INS_ldaddal, dataSize, dataReg, targetReg, addrReg);
break;
default:
assert(!"Unexpected treeNode->gtOper");
Expand Down
11 changes: 9 additions & 2 deletions src/coreclr/jit/lsraarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1086,10 +1086,17 @@ int LinearScan::BuildNode(GenTree* tree)
}
setInternalRegsDelayFree = true;
}
buildInternalRegisterUses();
if (dstCount == 1)
{
BuildDef(tree);
}
}
buildInternalRegisterUses();
if (dstCount == 1)
else
{
// We always need the target reg for LSE, even if
// return value is unused, see genLockedInstructions
buildInternalRegisterUses();
BuildDef(tree);
}
}
Expand Down