From c1a1c37980785fa81bb0d8f6aeea7692ccdce582 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Tue, 23 Jan 2024 10:03:11 -0400 Subject: [PATCH 1/5] OpcodeDispatcher: mark ideas to improve SHLD a bit tricky right now. Signed-off-by: Alyssa Rosenzweig --- FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp index 0658150d0..ee2189902 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp @@ -1734,7 +1734,13 @@ void OpDispatchBuilder::SHLDOp(OpcodeArgs) { const auto Size = GetSrcBitSize(Op); - // x86 masks the shift by 0x3F or 0x1F depending on size of op + // x86 masks the shift by 0x3F or 0x1F depending on size of op. Arm will mask + // by 0x3F when we do 64-bit shifts so we don't need to mask in that case, + // since the modulo is preserved even after presubtracting Size=64 for + // ShiftRight. + // + // TODO: Implement this optimization, it requires turning the shift=0 cases + // into (shift&0xc0) bit tests which is a bit complicated for now. if (Size == 64) { Shift = _And(OpSize::i64Bit, Shift, _Constant(0x3F)); } else { @@ -1751,6 +1757,12 @@ void OpDispatchBuilder::SHLDOp(OpcodeArgs) { // If shift count was zero then output doesn't change // Needs to be checked for the 32bit operand case // where shift = 0 and the source register still gets Zext + // + // TODO: With a backwards pass ahead-of-time, we could stick this in the + // if(shift) used for flags. + // + // TODO: This whole function wants to be wrapped in the if. Maybe b/w pass is + // a good idea after all. Res = _Select(FEXCore::IR::COND_EQ, Shift, _Constant(0), Dest, Res); From b3ae81f75fbb848fea76de7537e9bf73568420b2 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Tue, 23 Jan 2024 10:05:01 -0400 Subject: [PATCH 2/5] OpcodeDispatcher: allow garbage on shld shift Signed-off-by: Alyssa Rosenzweig --- FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp index ee2189902..2d371ea59 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp @@ -1730,7 +1730,8 @@ void OpDispatchBuilder::SHLDOp(OpcodeArgs) { OrderedNode *Src = LoadSource(GPRClass, Op, Op->Src[0], Op->Flags); OrderedNode *Dest = LoadSource(GPRClass, Op, Op->Dest, Op->Flags); - OrderedNode *Shift = LoadSource_WithOpSize(GPRClass, Op, Op->Src[1], 1, Op->Flags); + // Allow garbage on the shift, we're masking it anyway. + OrderedNode *Shift = LoadSource(GPRClass, Op, Op->Src[1], Op->Flags, {.AllowUpperGarbage = true}); const auto Size = GetSrcBitSize(Op); From 338f12845d3d78e1c6cc587a06769b770d4e52a2 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Tue, 23 Jan 2024 13:12:19 -0400 Subject: [PATCH 3/5] OpcodeDispatcher: save a constant in shld one weird trick Signed-off-by: Alyssa Rosenzweig --- FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp index 2d371ea59..ee25cf98b 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher.cpp @@ -1748,7 +1748,10 @@ void OpDispatchBuilder::SHLDOp(OpcodeArgs) { Shift = _And(OpSize::i64Bit, Shift, _Constant(0x1F)); } - auto ShiftRight = _Sub(OpSize::i64Bit, _Constant(Size), Shift); + // a64 masks the bottom bits, so if we're using a native 32/64-bit shift, we + // can negate to do the subtract (it's congruent), which saves a constant. + auto ShiftRight = Size >= 32 ? _Neg(OpSize::i64Bit, Shift) : + _Sub(OpSize::i64Bit, _Constant(Size), Shift); auto Tmp1 = _Lshl(OpSize::i64Bit, Dest, Shift); auto Tmp2 = _Lshr(Size == 64 ? OpSize::i64Bit : OpSize::i32Bit, Src, ShiftRight); From 0bc9e1a4097121422dd39333383074656baa5f86 Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Wed, 24 Jan 2024 22:50:13 -0400 Subject: [PATCH 4/5] OpcodeDispatcher: clobber OF with shift immediate Signed-off-by: Alyssa Rosenzweig --- FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp b/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp index d130c53fd..4b34a3318 100644 --- a/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp +++ b/FEXCore/Source/Interface/Core/OpcodeDispatcher/Flags.cpp @@ -817,8 +817,8 @@ void OpDispatchBuilder::CalculateFlags_SignShiftRightImmediate(uint8_t SrcSize, } void OpDispatchBuilder::CalculateFlags_ShiftRightImmediateCommon(uint8_t SrcSize, OrderedNode *Res, OrderedNode *Src1, uint64_t Shift) { - // Stash OF before overwriting it - auto OldOF = Shift != 1 ? GetRFLAG(FEXCore::X86State::RFLAG_OF_RAW_LOC) : NULL; + // Set SF and PF. Clobbers OF, but OF only defined for Shift = 1 where it is + // set below. SetNZ_ZeroCV(SrcSize, Res); // CF @@ -832,11 +832,6 @@ void OpDispatchBuilder::CalculateFlags_ShiftRightImmediateCommon(uint8_t SrcSize // AF // Undefined _InvalidateFlags(1 << X86State::RFLAG_AF_RAW_LOC); - - // Preserve OF if it won't be written - if (Shift != 1) { - SetRFLAG(OldOF); - } } void OpDispatchBuilder::CalculateFlags_ShiftRightImmediate(uint8_t SrcSize, OrderedNode *Res, OrderedNode *Src1, uint64_t Shift) { From 4cc59ba8f5c7b8218fdff376f43536e9f4bb01be Mon Sep 17 00:00:00 2001 From: Alyssa Rosenzweig Date: Tue, 30 Jan 2024 22:23:41 -0400 Subject: [PATCH 5/5] InstCountCI: Update Signed-off-by: Alyssa Rosenzweig --- .../FlagM/PrimaryGroup.json | 24 +++++--------- .../InstructionCountCI/FlagM/Secondary.json | 21 +++++------- .../InstructionCountCI/PrimaryGroup.json | 32 +++++++------------ unittests/InstructionCountCI/Secondary.json | 21 +++++------- 4 files changed, 36 insertions(+), 62 deletions(-) diff --git a/unittests/InstructionCountCI/FlagM/PrimaryGroup.json b/unittests/InstructionCountCI/FlagM/PrimaryGroup.json index 42390392e..8a55b250f 100644 --- a/unittests/InstructionCountCI/FlagM/PrimaryGroup.json +++ b/unittests/InstructionCountCI/FlagM/PrimaryGroup.json @@ -1113,16 +1113,14 @@ ] }, "shr al, 2": { - "ExpectedInstructionCount": 7, + "ExpectedInstructionCount": 5, "Comment": "GROUP2 0xC0 /5", "ExpectedArm64ASM": [ "uxtb w20, w4", "lsr w26, w20, #2", "bfxil x4, x26, #0, #8", - "cset w21, vs", "cmn wzr, w26, lsl #24", - "rmif x20, #0, #nzCv", - "rmif x21, #0, #nzcV" + "rmif x20, #0, #nzCv" ] }, "sar al, 2": { @@ -1331,42 +1329,36 @@ ] }, "shr ax, 2": { - "ExpectedInstructionCount": 7, + "ExpectedInstructionCount": 5, "Comment": "GROUP2 0xC1 /5", "ExpectedArm64ASM": [ "uxth w20, w4", "lsr w26, w20, #2", "bfxil x4, x26, #0, #16", - "cset w21, vs", "cmn wzr, w26, lsl #16", - "rmif x20, #0, #nzCv", - "rmif x21, #0, #nzcV" + "rmif x20, #0, #nzCv" ] }, "shr eax, 2": { - "ExpectedInstructionCount": 7, + "ExpectedInstructionCount": 5, "Comment": "GROUP2 0xC1 /5", "ExpectedArm64ASM": [ "mov w20, w4", "lsr w4, w20, #2", - "cset w21, vs", "tst w4, w4", "rmif x20, #0, #nzCv", - "mov x26, x4", - "rmif x21, #0, #nzcV" + "mov x26, x4" ] }, "shr rax, 2": { - "ExpectedInstructionCount": 7, + "ExpectedInstructionCount": 5, "Comment": "GROUP2 0xC1 /5", "ExpectedArm64ASM": [ "mov x20, x4", "lsr x4, x20, #2", - "cset w21, vs", "tst x4, x4", "rmif x20, #0, #nzCv", - "mov x26, x4", - "rmif x21, #0, #nzcV" + "mov x26, x4" ] }, "sar ax, 2": { diff --git a/unittests/InstructionCountCI/FlagM/Secondary.json b/unittests/InstructionCountCI/FlagM/Secondary.json index d0a30a9b1..8c173f88d 100644 --- a/unittests/InstructionCountCI/FlagM/Secondary.json +++ b/unittests/InstructionCountCI/FlagM/Secondary.json @@ -779,13 +779,12 @@ ] }, "shld ax, bx, cl": { - "ExpectedInstructionCount": 24, + "ExpectedInstructionCount": 23, "Comment": "0x0f 0xad", "ExpectedArm64ASM": [ "uxth w20, w7", "uxth w21, w4", - "uxtb w22, w5", - "and x22, x22, #0x1f", + "and x22, x5, #0x1f", "mov w23, #0x10", "sub x23, x23, x22", "lsl x24, x21, x22", @@ -809,15 +808,13 @@ ] }, "shld eax, ebx, cl": { - "ExpectedInstructionCount": 24, + "ExpectedInstructionCount": 22, "Comment": "0x0f 0xad", "ExpectedArm64ASM": [ "mov w20, w7", "mov w21, w4", - "uxtb w22, w5", - "and x22, x22, #0x1f", - "mov w23, #0x20", - "sub x23, x23, x22", + "and x22, x5, #0x1f", + "neg x23, x22", "lsl x24, x21, x22", "lsr w20, w20, w23", "orr x20, x24, x20", @@ -839,14 +836,12 @@ ] }, "shld rax, rbx, cl": { - "ExpectedInstructionCount": 22, + "ExpectedInstructionCount": 20, "Comment": "0x0f 0xad", "ExpectedArm64ASM": [ "mov x20, x4", - "uxtb w21, w5", - "and x21, x21, #0x3f", - "mov w22, #0x40", - "sub x22, x22, x21", + "and x21, x5, #0x3f", + "neg x22, x21", "lsl x23, x20, x21", "lsr x22, x7, x22", "orr x22, x23, x22", diff --git a/unittests/InstructionCountCI/PrimaryGroup.json b/unittests/InstructionCountCI/PrimaryGroup.json index 8c3ee4fc2..15ac89109 100644 --- a/unittests/InstructionCountCI/PrimaryGroup.json +++ b/unittests/InstructionCountCI/PrimaryGroup.json @@ -1239,18 +1239,16 @@ ] }, "shr al, 2": { - "ExpectedInstructionCount": 10, + "ExpectedInstructionCount": 8, "Comment": "GROUP2 0xC0 /5", "ExpectedArm64ASM": [ "uxtb w20, w4", "lsr w26, w20, #2", "bfxil x4, x26, #0, #8", - "cset w21, vs", "cmn wzr, w26, lsl #24", - "mrs x22, nzcv", + "mrs x21, nzcv", "ubfx x20, x20, #1, #1", - "orr w20, w22, w20, lsl #29", - "orr w20, w20, w21, lsl #28", + "orr w20, w21, w20, lsl #29", "msr nzcv, x20" ] }, @@ -1508,50 +1506,44 @@ ] }, "shr ax, 2": { - "ExpectedInstructionCount": 10, + "ExpectedInstructionCount": 8, "Comment": "GROUP2 0xC1 /5", "ExpectedArm64ASM": [ "uxth w20, w4", "lsr w26, w20, #2", "bfxil x4, x26, #0, #16", - "cset w21, vs", "cmn wzr, w26, lsl #16", - "mrs x22, nzcv", + "mrs x21, nzcv", "ubfx x20, x20, #1, #1", - "orr w20, w22, w20, lsl #29", - "orr w20, w20, w21, lsl #28", + "orr w20, w21, w20, lsl #29", "msr nzcv, x20" ] }, "shr eax, 2": { - "ExpectedInstructionCount": 10, + "ExpectedInstructionCount": 8, "Comment": "GROUP2 0xC1 /5", "ExpectedArm64ASM": [ "mov w20, w4", "lsr w4, w20, #2", - "cset w21, vs", "tst w4, w4", - "mrs x22, nzcv", + "mrs x21, nzcv", "ubfx x20, x20, #1, #1", - "orr w20, w22, w20, lsl #29", + "orr w20, w21, w20, lsl #29", "mov x26, x4", - "orr w20, w20, w21, lsl #28", "msr nzcv, x20" ] }, "shr rax, 2": { - "ExpectedInstructionCount": 10, + "ExpectedInstructionCount": 8, "Comment": "GROUP2 0xC1 /5", "ExpectedArm64ASM": [ "mov x20, x4", "lsr x4, x20, #2", - "cset w21, vs", "tst x4, x4", - "mrs x22, nzcv", + "mrs x21, nzcv", "ubfx x20, x20, #1, #1", - "orr w20, w22, w20, lsl #29", + "orr w20, w21, w20, lsl #29", "mov x26, x4", - "orr w20, w20, w21, lsl #28", "msr nzcv, x20" ] }, diff --git a/unittests/InstructionCountCI/Secondary.json b/unittests/InstructionCountCI/Secondary.json index 591b8751b..2c13ad430 100644 --- a/unittests/InstructionCountCI/Secondary.json +++ b/unittests/InstructionCountCI/Secondary.json @@ -1593,13 +1593,12 @@ ] }, "shld ax, bx, cl": { - "ExpectedInstructionCount": 28, + "ExpectedInstructionCount": 27, "Comment": "0x0f 0xad", "ExpectedArm64ASM": [ "uxth w20, w7", "uxth w21, w4", - "uxtb w22, w5", - "and x22, x22, #0x1f", + "and x22, x5, #0x1f", "mov w23, #0x10", "sub x23, x23, x22", "lsl x24, x21, x22", @@ -1627,15 +1626,13 @@ ] }, "shld eax, ebx, cl": { - "ExpectedInstructionCount": 28, + "ExpectedInstructionCount": 26, "Comment": "0x0f 0xad", "ExpectedArm64ASM": [ "mov w20, w7", "mov w21, w4", - "uxtb w22, w5", - "and x22, x22, #0x1f", - "mov w23, #0x20", - "sub x23, x23, x22", + "and x22, x5, #0x1f", + "neg x23, x22", "lsl x24, x21, x22", "lsr w20, w20, w23", "orr x20, x24, x20", @@ -1661,14 +1658,12 @@ ] }, "shld rax, rbx, cl": { - "ExpectedInstructionCount": 26, + "ExpectedInstructionCount": 24, "Comment": "0x0f 0xad", "ExpectedArm64ASM": [ "mov x20, x4", - "uxtb w21, w5", - "and x21, x21, #0x3f", - "mov w22, #0x40", - "sub x22, x22, x21", + "and x21, x5, #0x3f", + "neg x22, x21", "lsl x23, x20, x21", "lsr x22, x7, x22", "orr x22, x23, x22",