From bc37068f90d8a345f985856dbe60383dfcdc7ed2 Mon Sep 17 00:00:00 2001 From: "Herman S." <429230+has207@users.noreply.github.com> Date: Sat, 21 Mar 2026 14:15:58 +0900 Subject: [PATCH] [x64] Fix AVX-512 VECTOR_DENORMFLUSH merge-masking & FpRangeSign values Copy src1 into dest before the masked vrangeps so that unmasked lanes (normal/inf/NaN values) are preserved via merge-masking. Previously, when the register allocator assigned different physical registers for dest and src1, unmasked lanes would retain stale values from dest. Tested with Intel SDE and AVX-512 cpu to verify. Also shift unused FpRangeSign constants into bits [3:2] of the vrangeps immediate, matching the Intel encoding. Only one used in the code was 0 so it was already correct by accident but using the others in the future would produce incorrect results. --- src/xenia/cpu/backend/x64/x64_seq_vector.cc | 3 +- src/xenia/cpu/backend/x64/x64_util.h | 8 ++-- .../cpu/testing/vector_flush_denorm_test.cc | 41 +++++++++++++++++++ 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/src/xenia/cpu/backend/x64/x64_seq_vector.cc b/src/xenia/cpu/backend/x64/x64_seq_vector.cc index 30e8045aa..5e4b3b85c 100644 --- a/src/xenia/cpu/backend/x64/x64_seq_vector.cc +++ b/src/xenia/cpu/backend/x64/x64_seq_vector.cc @@ -165,7 +165,8 @@ struct VECTOR_DENORMFLUSH e.vptestnmd(denormal_mask, i.src1, e.GetXmmConstPtr(XMMSingleDenormalMask)); e.vxorps(e.xmm1, e.xmm1, e.xmm1); - e.vrangeps(i.dest.reg() | denormal_mask, i.src1, e.xmm1, + e.vmovaps(i.dest, i.src1); + e.vrangeps(i.dest.reg() | denormal_mask, i.dest, e.xmm1, FpRangeSelect::AbsMin | FpRangeSign::OperandA); return; } diff --git a/src/xenia/cpu/backend/x64/x64_util.h b/src/xenia/cpu/backend/x64/x64_util.h index 700841d96..9b4ad5049 100644 --- a/src/xenia/cpu/backend/x64/x64_util.h +++ b/src/xenia/cpu/backend/x64/x64_util.h @@ -47,10 +47,10 @@ constexpr uint8_t AbsMax = 0b11; // Larger absolute value }; // namespace FpRangeSelect namespace FpRangeSign { -constexpr uint8_t OperandA = 0b00; // Copy sign of operand A -constexpr uint8_t Preserve = 0b01; // Leave sign as is -constexpr uint8_t Positive = 0b10; // Set Positive -constexpr uint8_t Negative = 0b11; // Set Negative +constexpr uint8_t OperandA = 0b0000; // Copy sign of operand A +constexpr uint8_t Preserve = 0b0100; // Leave sign as is +constexpr uint8_t Positive = 0b1000; // Set Positive +constexpr uint8_t Negative = 0b1100; // Set Negative }; // namespace FpRangeSign } // namespace x64 diff --git a/src/xenia/cpu/testing/vector_flush_denorm_test.cc b/src/xenia/cpu/testing/vector_flush_denorm_test.cc index 138be31ce..ae30b2714 100644 --- a/src/xenia/cpu/testing/vector_flush_denorm_test.cc +++ b/src/xenia/cpu/testing/vector_flush_denorm_test.cc @@ -43,3 +43,44 @@ TEST_CASE("VECTOR_DENORMFLUSH_F32", "[instr]") { REQUIRE(result == vec128f(-FLT_MIN)); }); } + +// Test VECTOR_DENORMFLUSH with register pressure: input is kept +// alive past the denormflush, forcing dest and src into different +// physical registers. Verifies correct passthrough of normal values +// and preservation of the original input. +TEST_CASE("VECTOR_DENORMFLUSH_F32_REGPRESSURE", "[instr]") { + TestFunction test([](HIRBuilder& b) { + auto input = LoadVR(b, 4); + StoreVR(b, 3, b.VectorDenormFlush(input)); + StoreVR(b, 5, input); // keep input alive past denormflush + b.Return(); + }); + // Mix of denormal and normal elements: + // [0] 0x007FFFFF = positive denormal -> should flush to +0.0 + // [1] 0x3F800000 = 1.0f (normal) -> should pass through + // [2] 0x807FFFFF = negative denormal -> should flush to -0.0 + // [3] 0x40000000 = 2.0f (normal) -> should pass through + test.Run( + [](PPCContext* ctx) { + ctx->v[4] = vec128i(0x007FFFFF, 0x3F800000, 0x807FFFFF, 0x40000000); + }, + [](PPCContext* ctx) { + auto result = ctx->v[3]; + REQUIRE(result == + vec128i(0x00000000, 0x3F800000, 0x80000000, 0x40000000)); + // Original input must also be preserved + REQUIRE(ctx->v[5] == + vec128i(0x007FFFFF, 0x3F800000, 0x807FFFFF, 0x40000000)); + }); + // All normal values - with the bug, ALL lanes would be corrupted + // since k1 would be all-zero and merge-masking preserves dest, not src1 + test.Run( + [](PPCContext* ctx) { + ctx->v[4] = vec128i(0x3F800000, 0x40000000, 0x40400000, 0x40800000); + }, + [](PPCContext* ctx) { + auto result = ctx->v[3]; + REQUIRE(result == + vec128i(0x3F800000, 0x40000000, 0x40400000, 0x40800000)); + }); +}