From fc02f38435d5755aa028c16d883004185fe5ba4c Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Fri, 15 Sep 2023 12:08:22 -0400 Subject: [PATCH 1/2] IR: Only invert CF for NZCV if needed If we are going to throw away the updated value of CF anyway there is no point wasting an instruction to invert CF. Add an IR toggle for that so the arm64 JIT can make better choices. Signed-off-by: Alyssa Rosenzweig --- .../Source/Interface/Core/Interpreter/ALUOps.cpp | 4 ++-- .../Source/Interface/Core/JIT/Arm64/ALUOps.cpp | 16 +++++++++------- .../Source/Interface/Core/JIT/x86_64/ALUOps.cpp | 4 ++++ .../Interface/Core/OpcodeDispatcher/Flags.cpp | 3 ++- FEXCore/Source/Interface/IR/IR.json | 4 ++-- 5 files changed, 19 insertions(+), 12 deletions(-) diff --git a/FEXCore/Source/Interface/Core/Interpreter/ALUOps.cpp b/FEXCore/Source/Interface/Core/Interpreter/ALUOps.cpp index 3d4469c0f..54cbb64b0 100644 --- a/FEXCore/Source/Interface/Core/Interpreter/ALUOps.cpp +++ b/FEXCore/Source/Interface/Core/Interpreter/ALUOps.cpp @@ -205,7 +205,7 @@ DEF_OP(SubNZCV) { if (Result == 0) { NZCV |= 1U << 30; } - if (__builtin_usub_overflow(Src1, Src2, &Result)) { + if (__builtin_usub_overflow(Src1, Src2, &Result) ^ !(Op->InvertCarry)) { NZCV |= 1U << 29; } if (__builtin_ssub_overflow(Src1, Src2, &ResultSigned)) { @@ -222,7 +222,7 @@ DEF_OP(SubNZCV) { if (Result == 0) { NZCV |= 1U << 30; } - if (__builtin_usubl_overflow(Src1, Src2, &Result)) { + if (__builtin_usubl_overflow(Src1, Src2, &Result) ^ !(Op->InvertCarry)) { NZCV |= 1U << 29; } if (__builtin_ssubl_overflow(Src1, Src2, &ResultSigned)) { diff --git a/FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp b/FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp index 380a98474..7959f7738 100644 --- a/FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp +++ b/FEXCore/Source/Interface/Core/JIT/Arm64/ALUOps.cpp @@ -154,13 +154,15 @@ DEF_OP(SubNZCV) { // TODO: Optimize this out mrs(Dst, ARMEmitter::SystemRegister::NZCV); - // The carry flag produced by arm64 subs is inverted compared to the x86 carry - // flag. Invert it now. - // - // TODO: Once we optimize out the mrs, this will become a cfinv operation, but - // that's only available with Feat_FlagM. For now the portable way is to flip - // bit 29 (carry) manually. - eor(ARMEmitter::Size::i32Bit, Dst, Dst, 1u << 29); + if (Op->InvertCarry) { + // The carry flag produced by arm64 subs is inverted compared to the x86 carry + // flag. Invert it now. + // + // TODO: Once we optimize out the mrs, this will become a cfinv operation, but + // that's only available with Feat_FlagM. For now the portable way is to flip + // bit 29 (carry) manually. + eor(ARMEmitter::Size::i32Bit, Dst, Dst, 1u << 29); + } } DEF_OP(Neg) { diff --git a/FEXCore/Source/Interface/Core/JIT/x86_64/ALUOps.cpp b/FEXCore/Source/Interface/Core/JIT/x86_64/ALUOps.cpp index 95ff9a61c..411403642 100644 --- a/FEXCore/Source/Interface/Core/JIT/x86_64/ALUOps.cpp +++ b/FEXCore/Source/Interface/Core/JIT/x86_64/ALUOps.cpp @@ -262,6 +262,10 @@ DEF_OP(SubNZCV) { break; } + if (!Op->InvertCarry) { + cmc(); + } + mov(TMP1, 0); mov(TMP2, 0); mov(TMP3, 0); diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp index 518906cfe..cb9c2587f 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp @@ -533,7 +533,8 @@ void OpDispatchBuilder::CalculateFlags_SUB(uint8_t SrcSize, OrderedNode *Res, Or // TODO: Could do this path for small sources if we have FEAT_FlagM if (SrcSize >= 4) { - SetNZCV(_SubNZCV(OpSize, Src1, Src2)); + // We only bother inverting CF if we're actually going to update CF. + SetNZCV(_SubNZCV(OpSize, Src1, Src2, UpdateCF)); } else { // SF/ZF SetNZ_ZeroCV(SrcSize, Res); diff --git a/FEXCore/Source/Interface/IR/IR.json b/FEXCore/Source/Interface/IR/IR.json index c0bf0f447..a21f76f89 100644 --- a/FEXCore/Source/Interface/IR/IR.json +++ b/FEXCore/Source/Interface/IR/IR.json @@ -901,9 +901,9 @@ "Size == FEXCore::IR::OpSize::i32Bit || Size == FEXCore::IR::OpSize::i64Bit" ] }, - "GPR = SubNZCV OpSize:$Size, GPR:$Src1, GPR:$Src2": { + "GPR = SubNZCV OpSize:$Size, GPR:$Src1, GPR:$Src2, u8:$InvertCarry": { "Desc": ["Return NZCV for the difference of two GPRs. ", - "Note: Carry flag uses x86 definition, inverted from arm64.", + "If InvertCarry is nonzero, carry flag uses x86 definition, inverted from arm64.", ""], "DestSize": "4", "EmitValidation": [ From c560a88de4b2f1e8b305c9ca8f1f9e6f460a9c4e Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Fri, 15 Sep 2023 12:10:57 -0400 Subject: [PATCH 2/2] InstCountCI: Update Signed-off-by: Alyssa Rosenzweig --- unittests/InstructionCountCI/PrimaryGroup.json | 6 ++---- unittests/InstructionCountCI/Primary_32Bit.json | 3 +-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/unittests/InstructionCountCI/PrimaryGroup.json b/unittests/InstructionCountCI/PrimaryGroup.json index 713a35cce..6af0c85c8 100644 --- a/unittests/InstructionCountCI/PrimaryGroup.json +++ b/unittests/InstructionCountCI/PrimaryGroup.json @@ -3479,7 +3479,7 @@ ] }, "dec eax": { - "ExpectedInstructionCount": 12, + "ExpectedInstructionCount": 11, "Optimal": "No", "Comment": "GROUP4 0xfe /1", "ExpectedArm64ASM": [ @@ -3492,13 +3492,12 @@ "ubfx w21, w21, #29, #1", "cmp w20, #0x1 (1)", "mrs x20, nzcv", - "eor w20, w20, #0x20000000", "bfi w20, w21, #29, #1", "str w20, [x28, #728]" ] }, "dec rax": { - "ExpectedInstructionCount": 12, + "ExpectedInstructionCount": 11, "Optimal": "Yes", "Comment": "GROUP4 0xfe /1", "ExpectedArm64ASM": [ @@ -3511,7 +3510,6 @@ "ubfx w21, w21, #29, #1", "cmp x20, #0x1 (1)", "mrs x20, nzcv", - "eor w20, w20, #0x20000000", "bfi w20, w21, #29, #1", "str w20, [x28, #728]" ] diff --git a/unittests/InstructionCountCI/Primary_32Bit.json b/unittests/InstructionCountCI/Primary_32Bit.json index c76be73ff..b958672c8 100644 --- a/unittests/InstructionCountCI/Primary_32Bit.json +++ b/unittests/InstructionCountCI/Primary_32Bit.json @@ -360,7 +360,7 @@ ] }, "dec eax": { - "ExpectedInstructionCount": 12, + "ExpectedInstructionCount": 11, "Optimal": "No", "Comment": "0x48", "ExpectedArm64ASM": [ @@ -373,7 +373,6 @@ "ubfx w21, w21, #29, #1", "cmp w20, #0x1 (1)", "mrs x20, nzcv", - "eor w20, w20, #0x20000000", "bfi w20, w21, #29, #1", "str w20, [x28, #728]" ]