From 814e37824921e10d020c9e957db0b064c90f469b Mon Sep 17 00:00:00 2001 From: MerryMage Date: Wed, 22 Nov 2017 17:45:37 +0000 Subject: [PATCH] VCMP and VCMPE were the other way around - This was due to a misunderstanding of what the E in VCMPE means. - The E refers to an exception being raised when a QNaN is encountered. - Added unit tests for VCMP{E} --- src/backend_x64/emit_x64.cpp | 16 ++++++++-------- src/backend_x64/jitstate.cpp | 2 +- src/frontend/ir/ir_emitter.cpp | 8 ++++---- src/frontend/ir/ir_emitter.h | 4 ++-- src/frontend/translate/translate_arm/vfp2.cpp | 12 ++++++------ tests/arm/fuzz_arm.cpp | 12 +++++++++++- 6 files changed, 32 insertions(+), 22 deletions(-) diff --git a/src/backend_x64/emit_x64.cpp b/src/backend_x64/emit_x64.cpp index 4d7e732c..97dade90 100644 --- a/src/backend_x64/emit_x64.cpp +++ b/src/backend_x64/emit_x64.cpp @@ -2473,12 +2473,12 @@ void EmitX64::EmitFPCompare32(RegAlloc& reg_alloc, IR::Block&, IR::Inst* inst) { auto args = reg_alloc.GetArgumentInfo(inst); Xbyak::Xmm reg_a = reg_alloc.UseXmm(args[0]); Xbyak::Xmm reg_b = reg_alloc.UseXmm(args[1]); - bool quiet = args[2].GetImmediateU1(); + bool exc_on_qnan = args[2].GetImmediateU1(); - if (quiet) { - code->ucomiss(reg_a, reg_b); - } else { + if (exc_on_qnan) { code->comiss(reg_a, reg_b); + } else { + code->ucomiss(reg_a, reg_b); } SetFpscrNzcvFromFlags(code, reg_alloc); @@ -2488,12 +2488,12 @@ void EmitX64::EmitFPCompare64(RegAlloc& reg_alloc, IR::Block&, IR::Inst* inst) { auto args = reg_alloc.GetArgumentInfo(inst); Xbyak::Xmm reg_a = reg_alloc.UseXmm(args[0]); Xbyak::Xmm reg_b = reg_alloc.UseXmm(args[1]); - bool quiet = args[2].GetImmediateU1(); + bool exc_on_qnan = args[2].GetImmediateU1(); - if (quiet) { - code->ucomisd(reg_a, reg_b); - } else { + if (exc_on_qnan) { code->comisd(reg_a, reg_b); + } else { + code->ucomisd(reg_a, reg_b); } SetFpscrNzcvFromFlags(code, reg_alloc); diff --git a/src/backend_x64/jitstate.cpp b/src/backend_x64/jitstate.cpp index 98b33df3..1d01c4a2 100644 --- a/src/backend_x64/jitstate.cpp +++ b/src/backend_x64/jitstate.cpp @@ -120,7 +120,7 @@ void JitState::SetFpscr(u32 FPSCR) { if (Common::Bit<24>(FPSCR)) { // VFP Flush to Zero //guest_MXCSR |= (1 << 15); // SSE Flush to Zero - guest_MXCSR |= (1 << 6); // SSE Denormals are Zero + //guest_MXCSR |= (1 << 6); // SSE Denormals are Zero } } diff --git a/src/frontend/ir/ir_emitter.cpp b/src/frontend/ir/ir_emitter.cpp index fa4caaf9..cd0c8c86 100644 --- a/src/frontend/ir/ir_emitter.cpp +++ b/src/frontend/ir/ir_emitter.cpp @@ -544,14 +544,14 @@ Value IREmitter::FPAdd64(const Value& a, const Value& b, bool fpscr_controlled) return Inst(Opcode::FPAdd64, {a, b}); } -void IREmitter::FPCompare32(const Value& a, const Value& b, bool quiet, bool fpscr_controlled) { +void IREmitter::FPCompare32(const Value& a, const Value& b, bool exc_on_qnan, bool fpscr_controlled) { ASSERT(fpscr_controlled); - Inst(Opcode::FPCompare32, {a, b, Imm1(quiet)}); + Inst(Opcode::FPCompare32, {a, b, Imm1(exc_on_qnan)}); } -void IREmitter::FPCompare64(const Value& a, const Value& b, bool quiet, bool fpscr_controlled) { +void IREmitter::FPCompare64(const Value& a, const Value& b, bool exc_on_qnan, bool fpscr_controlled) { ASSERT(fpscr_controlled); - Inst(Opcode::FPCompare64, {a, b, Imm1(quiet)}); + Inst(Opcode::FPCompare64, {a, b, Imm1(exc_on_qnan)}); } Value IREmitter::FPDiv32(const Value& a, const Value& b, bool fpscr_controlled) { diff --git a/src/frontend/ir/ir_emitter.h b/src/frontend/ir/ir_emitter.h index be341e0e..74c1f784 100644 --- a/src/frontend/ir/ir_emitter.h +++ b/src/frontend/ir/ir_emitter.h @@ -183,8 +183,8 @@ public: Value FPAbs64(const Value& a); Value FPAdd32(const Value& a, const Value& b, bool fpscr_controlled); Value FPAdd64(const Value& a, const Value& b, bool fpscr_controlled); - void FPCompare32(const Value& a, const Value& b, bool quiet, bool fpscr_controlled); - void FPCompare64(const Value& a, const Value& b, bool quiet, bool fpscr_controlled); + void FPCompare32(const Value& a, const Value& b, bool exc_on_qnan, bool fpscr_controlled); + void FPCompare64(const Value& a, const Value& b, bool exc_on_qnan, bool fpscr_controlled); Value FPDiv32(const Value& a, const Value& b, bool fpscr_controlled); Value FPDiv64(const Value& a, const Value& b, bool fpscr_controlled); Value FPMul32(const Value& a, const Value& b, bool fpscr_controlled); diff --git a/src/frontend/translate/translate_arm/vfp2.cpp b/src/frontend/translate/translate_arm/vfp2.cpp index def85f5f..1f6d3285 100644 --- a/src/frontend/translate/translate_arm/vfp2.cpp +++ b/src/frontend/translate/translate_arm/vfp2.cpp @@ -488,16 +488,16 @@ bool ArmTranslatorVisitor::vfp2_VCVT_to_s32(Cond cond, bool D, size_t Vd, bool s bool ArmTranslatorVisitor::vfp2_VCMP(Cond cond, bool D, size_t Vd, bool sz, bool E, bool M, size_t Vm) { ExtReg d = ToExtReg(sz, Vd, D); ExtReg m = ToExtReg(sz, Vm, M); - bool quiet = E; + bool exc_on_qnan = E; // VCMP{E}.F32 , // VCMP{E}.F64
, if (ConditionPassed(cond)) { auto reg_d = ir.GetExtendedRegister(d); auto reg_m = ir.GetExtendedRegister(m); if (sz) { - ir.FPCompare64(reg_d, reg_m, quiet, true); + ir.FPCompare64(reg_d, reg_m, exc_on_qnan, true); } else { - ir.FPCompare32(reg_d, reg_m, quiet, true); + ir.FPCompare32(reg_d, reg_m, exc_on_qnan, true); } } return true; @@ -505,7 +505,7 @@ bool ArmTranslatorVisitor::vfp2_VCMP(Cond cond, bool D, size_t Vd, bool sz, bool bool ArmTranslatorVisitor::vfp2_VCMP_zero(Cond cond, bool D, size_t Vd, bool sz, bool E) { ExtReg d = ToExtReg(sz, Vd, D); - bool quiet = E; + bool exc_on_qnan = E; // VCMP{E}.F32 , #0.0 // VCMP{E}.F64
, #0.0 if (ConditionPassed(cond)) { @@ -514,9 +514,9 @@ bool ArmTranslatorVisitor::vfp2_VCMP_zero(Cond cond, bool D, size_t Vd, bool sz, ? ir.TransferToFP64(ir.Imm64(0)) : ir.TransferToFP32(ir.Imm32(0)); if (sz) { - ir.FPCompare64(reg_d, zero, quiet, true); + ir.FPCompare64(reg_d, zero, exc_on_qnan, true); } else { - ir.FPCompare32(reg_d, zero, quiet, true); + ir.FPCompare32(reg_d, zero, exc_on_qnan, true); } } return true; diff --git a/tests/arm/fuzz_arm.cpp b/tests/arm/fuzz_arm.cpp index 5a55545a..a66e1f42 100644 --- a/tests/arm/fuzz_arm.cpp +++ b/tests/arm/fuzz_arm.cpp @@ -535,7 +535,6 @@ TEST_CASE("VFP: VMOV", "[JitX64][vfp]") { }); } - TEST_CASE("VFP: VMOV (reg), VLDR, VSTR", "[JitX64][vfp]") { const std::array instructions = {{ InstructionGenerator("1111000100000001000000e000000000"), // SETEND @@ -549,6 +548,17 @@ TEST_CASE("VFP: VMOV (reg), VLDR, VSTR", "[JitX64][vfp]") { }); } +TEST_CASE("VFP: VCMP", "[JitX64][vfp]") { + const std::array instructions = {{ + InstructionGenerator("cccc11101D110100dddd101zE1M0mmmm"), // VCMP + InstructionGenerator("cccc11101D110101dddd101zE1000000"), // VCMP (zero) + }}; + + FuzzJitArm(5, 6, 10000, [&instructions]() -> u32 { + return instructions[RandInt(0, instructions.size() - 1)].Generate(); + }); +} + TEST_CASE("Fuzz ARM data processing instructions", "[JitX64]") { const std::array imm_instructions = {{ InstructionGenerator("cccc0010101Snnnnddddrrrrvvvvvvvv"),