From cd124716d2d2f9068c8dd91c37a4c058b03c0853 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 27 Jan 2024 14:21:08 +0100 Subject: [PATCH 1/4] fix GT_XCHG for byte on x86 --- src/coreclr/jit/lsraxarch.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index 9912598f19553c..a4e5bc823b153c 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -466,7 +466,8 @@ int LinearScan::BuildNode(GenTree* tree) BuildUse(data); srcCount = 2; assert(dstCount == 1); - BuildDef(tree); + const bool needsByteRegs = (TARGET_POINTER_SIZE == 4) && varTypeIsByte(tree); + BuildDef(tree, needsByteRegs ? RBM_BYTE_REGS : RBM_NONE); } break; From 7b68b622fc8973b944d6b005801a98d639b61c05 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 27 Jan 2024 15:56:15 +0100 Subject: [PATCH 2/4] Correct fix --- src/coreclr/jit/lsraxarch.cpp | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index a4e5bc823b153c..cbb6041a8f6eb2 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -425,11 +425,17 @@ int LinearScan::BuildNode(GenTree* tree) srcCount = 3; assert(dstCount == 1); + GenTree* addr = tree->AsCmpXchg()->Addr(); + GenTree* data = tree->AsCmpXchg()->Data(); + GenTree* comparand = tree->AsCmpXchg()->Comparand(); + // Comparand is preferenced to RAX. // The remaining two operands can be in any reg other than RAX. - BuildUse(tree->AsCmpXchg()->Addr(), availableIntRegs & ~RBM_RAX); - BuildUse(tree->AsCmpXchg()->Data(), availableIntRegs & ~RBM_RAX); - BuildUse(tree->AsCmpXchg()->Comparand(), RBM_RAX); + + const unsigned nonRaxCandidates = availableIntRegs & ~RBM_RAX; + BuildUse(addr, nonRaxCandidates); + BuildUse(data, varTypeIsByte(data) ? (nonRaxCandidates & RBM_BYTE_REGS) : nonRaxCandidates); + BuildUse(comparand, RBM_RAX); BuildDef(tree, RBM_RAX); } break; @@ -438,10 +444,16 @@ int LinearScan::BuildNode(GenTree* tree) case GT_XAND: if (!tree->IsUnusedValue()) { + GenTree* addr = tree->gtGetOp1(); + GenTree* data = tree->gtGetOp2(); + + // These don't support byte operands. + assert(!varTypeIsByte(data)); + // if tree's value is used, we'll emit a cmpxchg-loop idiom (requires RAX) buildInternalIntRegisterDefForNode(tree, availableIntRegs & ~RBM_RAX); - BuildUse(tree->gtGetOp1(), availableIntRegs & ~RBM_RAX); - BuildUse(tree->gtGetOp2(), availableIntRegs & ~RBM_RAX); + BuildUse(addr, availableIntRegs & ~RBM_RAX); + BuildUse(data, availableIntRegs & ~RBM_RAX); BuildDef(tree, RBM_RAX); buildInternalRegisterUses(); srcCount = 2; @@ -463,11 +475,14 @@ int LinearScan::BuildNode(GenTree* tree) setDelayFree(addrUse); tgtPrefUse = addrUse; assert(!data->isContained()); - BuildUse(data); + + // Codegen will need data to be in the target reg, so we're requesting byteable registers + // if either tree or data is a byte type, e.g. tree is TYP_UBYTE and data is TYP_INT (cns) + // codegen will emit a mov from the data reg to the target reg. + BuildUse(data, (varTypeIsByte(data) || varTypeIsByte(tree)) ? RBM_BYTE_REGS : RBM_ALLINT); srcCount = 2; assert(dstCount == 1); - const bool needsByteRegs = (TARGET_POINTER_SIZE == 4) && varTypeIsByte(tree); - BuildDef(tree, needsByteRegs ? RBM_BYTE_REGS : RBM_NONE); + BuildDef(tree); } break; From a54ab3d47583b4ae52654594c5d00a09ece8ba5b Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 27 Jan 2024 16:09:52 +0100 Subject: [PATCH 3/4] Same for GT_CMPXCHG --- src/coreclr/jit/lsraxarch.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index cbb6041a8f6eb2..0b93698acd6cbd 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -434,7 +434,8 @@ int LinearScan::BuildNode(GenTree* tree) const unsigned nonRaxCandidates = availableIntRegs & ~RBM_RAX; BuildUse(addr, nonRaxCandidates); - BuildUse(data, varTypeIsByte(data) ? (nonRaxCandidates & RBM_BYTE_REGS) : nonRaxCandidates); + BuildUse(data, (varTypeIsByte(tree) || varTypeIsByte(data)) ? (nonRaxCandidates & RBM_BYTE_REGS) + : nonRaxCandidates); BuildUse(comparand, RBM_RAX); BuildDef(tree, RBM_RAX); } @@ -475,10 +476,6 @@ int LinearScan::BuildNode(GenTree* tree) setDelayFree(addrUse); tgtPrefUse = addrUse; assert(!data->isContained()); - - // Codegen will need data to be in the target reg, so we're requesting byteable registers - // if either tree or data is a byte type, e.g. tree is TYP_UBYTE and data is TYP_INT (cns) - // codegen will emit a mov from the data reg to the target reg. BuildUse(data, (varTypeIsByte(data) || varTypeIsByte(tree)) ? RBM_BYTE_REGS : RBM_ALLINT); srcCount = 2; assert(dstCount == 1); From b19632f7b72fa9fdf77cca17644ef23023dc7010 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 27 Jan 2024 16:42:06 +0100 Subject: [PATCH 4/4] Simpler fix --- src/coreclr/jit/lsraxarch.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index 0b93698acd6cbd..33122e79777b95 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -434,8 +434,7 @@ int LinearScan::BuildNode(GenTree* tree) const unsigned nonRaxCandidates = availableIntRegs & ~RBM_RAX; BuildUse(addr, nonRaxCandidates); - BuildUse(data, (varTypeIsByte(tree) || varTypeIsByte(data)) ? (nonRaxCandidates & RBM_BYTE_REGS) - : nonRaxCandidates); + BuildUse(data, varTypeIsByte(tree) ? (nonRaxCandidates & RBM_BYTE_REGS) : nonRaxCandidates); BuildUse(comparand, RBM_RAX); BuildDef(tree, RBM_RAX); } @@ -476,7 +475,7 @@ int LinearScan::BuildNode(GenTree* tree) setDelayFree(addrUse); tgtPrefUse = addrUse; assert(!data->isContained()); - BuildUse(data, (varTypeIsByte(data) || varTypeIsByte(tree)) ? RBM_BYTE_REGS : RBM_ALLINT); + BuildUse(data, varTypeIsByte(tree) ? RBM_BYTE_REGS : RBM_NONE); srcCount = 2; assert(dstCount == 1); BuildDef(tree);