From ff25e9a92e9186ef346d84ad62a2389b09567eac Mon Sep 17 00:00:00 2001 From: Ryan Houdek Date: Tue, 11 Nov 2025 17:16:03 -0800 Subject: [PATCH] FEXCore: Remove usage of "remote atomic" xor This is the only usage of LSE atomics that isn't the fetch variety. [This article](https://www.phoronix.com/news/Linux-6.18-ARM64-Atomics-Issue) reminded me that this was a thing and that I should double check the IR. This was the only IR operation remaining that still didn't use the fetch variety. Convert it over to the fetch to avoid the expectation that it can be a "remote atomic". Change is going to fall in to noise, but might as well as be consistent. --- .../Source/Interface/Core/JIT/AtomicOps.cpp | 20 ------------------- .../Interface/Core/OpcodeDispatcher.cpp | 3 ++- FEXCore/Source/Interface/IR/IR.json | 10 ---------- unittests/InstructionCountCI/Atomics.json | 8 ++++---- .../InstructionCountCI/FlagM/Atomics.json | 8 ++++---- 5 files changed, 10 insertions(+), 39 deletions(-) diff --git a/FEXCore/Source/Interface/Core/JIT/AtomicOps.cpp b/FEXCore/Source/Interface/Core/JIT/AtomicOps.cpp index 51ea83ec1..e819e8161 100644 --- a/FEXCore/Source/Interface/Core/JIT/AtomicOps.cpp +++ b/FEXCore/Source/Interface/Core/JIT/AtomicOps.cpp @@ -138,26 +138,6 @@ DEF_OP(CAS) { } } -DEF_OP(AtomicXor) { - auto Op = IROp->C(); - const auto EmitSize = ConvertSize(IROp); - const auto SubEmitSize = ConvertSubRegSize8(IROp->Size); - - auto MemSrc = GetReg(Op->Addr); - auto Src = GetReg(Op->Value); - - if (CTX->HostFeatures.SupportsAtomics) { - steorl(SubEmitSize, Src, MemSrc); - } else { - ARMEmitter::BackwardLabel LoopTop; - (void)Bind(&LoopTop); - ldaxr(SubEmitSize, TMP2, MemSrc); - eor(EmitSize, TMP2, TMP2, Src); - stlxr(SubEmitSize, TMP2, TMP2, MemSrc); - (void)cbnz(EmitSize, TMP2, &LoopTop); - } -} - DEF_OP(AtomicSwap) { auto Op = IROp->C(); const auto OpSize = IROp->Size; diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp index 5f6f677d0..2736e2288 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp @@ -2750,7 +2750,8 @@ void OpDispatchBuilder::NOTOp(OpcodeArgs) { if (DestIsLockedMem(Op)) { HandledLock = true; Ref DestMem = MakeSegmentAddress(Op, Op->Dest); - _AtomicXor(Size, MaskConst, DestMem); + // Result unused + _AtomicFetchXor(Size, MaskConst, DestMem); } else if (!Op->Dest.IsGPR()) { // GPR version plays fast and loose with sizes, be safe for memory tho. Ref Src = LoadSourceGPR(Op, Op->Dest, Op->Flags); diff --git a/FEXCore/Source/Interface/IR/IR.json b/FEXCore/Source/Interface/IR/IR.json index 0a02cbd0f..f15ea967a 100644 --- a/FEXCore/Source/Interface/IR/IR.json +++ b/FEXCore/Source/Interface/IR/IR.json @@ -804,16 +804,6 @@ "Size == FEXCore::IR::OpSize::i32Bit || Size == FEXCore::IR::OpSize::i64Bit" ] }, - "AtomicXor OpSize:#Size, GPR:$Value, GPR:$Addr": { - "HasSideEffects": true, - "Desc": ["Atomic integer xor", - "IR layout must match Fetch-variant, otherwise DCE IR optimization breaks!" - ], - "DestSize": "Size", - "EmitValidation": [ - "Size == FEXCore::IR::OpSize::i8Bit || Size == FEXCore::IR::OpSize::i16Bit || Size == FEXCore::IR::OpSize::i32Bit || Size == FEXCore::IR::OpSize::i64Bit" - ] - }, "GPR = AtomicSwap OpSize:#Size, GPR:$Value, GPR:$Addr": { "HasSideEffects": true, "Desc": ["Atomic integer swap" diff --git a/unittests/InstructionCountCI/Atomics.json b/unittests/InstructionCountCI/Atomics.json index d04513f25..06c19cc6a 100644 --- a/unittests/InstructionCountCI/Atomics.json +++ b/unittests/InstructionCountCI/Atomics.json @@ -1491,7 +1491,7 @@ "Comment": "GROUP2 0xf6 /2", "ExpectedArm64ASM": [ "mov w20, #0xff", - "steorlb w20, [x4]" + "ldeoralb w20, w20, [x4]" ] }, "lock not word [rax]": { @@ -1499,7 +1499,7 @@ "Comment": "GROUP2 0xf7 /2", "ExpectedArm64ASM": [ "mov w20, #0xffff", - "steorlh w20, [x4]" + "ldeoralh w20, w20, [x4]" ] }, "lock not dword [rax]": { @@ -1507,7 +1507,7 @@ "Comment": "GROUP2 0xf7 /2", "ExpectedArm64ASM": [ "mov w20, #0xffffffff", - "steorl w20, [x4]" + "ldeoral w20, w20, [x4]" ] }, "lock not qword [rax]": { @@ -1515,7 +1515,7 @@ "Comment": "GROUP2 0xf7 /2", "ExpectedArm64ASM": [ "mov x20, #0xffffffffffffffff", - "steorl x20, [x4]" + "ldeoral x20, x20, [x4]" ] }, "lock neg byte [rax]": { diff --git a/unittests/InstructionCountCI/FlagM/Atomics.json b/unittests/InstructionCountCI/FlagM/Atomics.json index ea958017b..4407ee9bc 100644 --- a/unittests/InstructionCountCI/FlagM/Atomics.json +++ b/unittests/InstructionCountCI/FlagM/Atomics.json @@ -1294,7 +1294,7 @@ "Comment": "GROUP2 0xf6 /2", "ExpectedArm64ASM": [ "mov w20, #0xff", - "steorlb w20, [x4]" + "ldeoralb w20, w20, [x4]" ] }, "lock not word [rax]": { @@ -1302,7 +1302,7 @@ "Comment": "GROUP2 0xf7 /2", "ExpectedArm64ASM": [ "mov w20, #0xffff", - "steorlh w20, [x4]" + "ldeoralh w20, w20, [x4]" ] }, "lock not dword [rax]": { @@ -1310,7 +1310,7 @@ "Comment": "GROUP2 0xf7 /2", "ExpectedArm64ASM": [ "mov w20, #0xffffffff", - "steorl w20, [x4]" + "ldeoral w20, w20, [x4]" ] }, "lock not qword [rax]": { @@ -1318,7 +1318,7 @@ "Comment": "GROUP2 0xf7 /2", "ExpectedArm64ASM": [ "mov x20, #0xffffffffffffffff", - "steorl x20, [x4]" + "ldeoral x20, x20, [x4]" ] }, "lock neg byte [rax]": {