From bb70e5c6512114b28f599ab535eb9cf8e833aa92 Mon Sep 17 00:00:00 2001 From: "Herman S." <429230+has207@users.noreply.github.com> Date: Thu, 12 Feb 2026 09:31:54 +0900 Subject: [PATCH] [CPU] Fix incorrect all_same detection in constant vector shift paths The loop conditions `n < 8 - n` and `n < 4 - n` terminated early, only checking the first half of elements. This caused EmitInt16 and EmitInt32 to incorrectly take the uniform shift path when trailing elements had different shift amounts. Resolves potential issues in SHL, SHR, and SHA. --- src/xenia/cpu/backend/x64/x64_seq_vector.cc | 12 ++--- src/xenia/cpu/testing/vector_sha_test.cc | 41 +++++++++++++++++ src/xenia/cpu/testing/vector_shl_test.cc | 51 +++++++++++++++++++++ src/xenia/cpu/testing/vector_shr_test.cc | 41 +++++++++++++++++ 4 files changed, 139 insertions(+), 6 deletions(-) diff --git a/src/xenia/cpu/backend/x64/x64_seq_vector.cc b/src/xenia/cpu/backend/x64/x64_seq_vector.cc index 029eabb3b..d72cd903a 100644 --- a/src/xenia/cpu/backend/x64/x64_seq_vector.cc +++ b/src/xenia/cpu/backend/x64/x64_seq_vector.cc @@ -1057,7 +1057,7 @@ struct VECTOR_SHL_V128 if (i.src2.is_constant) { const auto& shamt = i.src2.constant(); bool all_same = true; - for (size_t n = 0; n < 8 - n; ++n) { + for (size_t n = 0; n < 7; ++n) { if (shamt.u16[n] != shamt.u16[n + 1]) { all_same = false; break; @@ -1135,7 +1135,7 @@ struct VECTOR_SHL_V128 if (i.src2.is_constant) { const auto& shamt = i.src2.constant(); bool all_same = true; - for (size_t n = 0; n < 4 - n; ++n) { + for (size_t n = 0; n < 3; ++n) { if (shamt.u32[n] != shamt.u32[n + 1]) { all_same = false; break; @@ -1335,7 +1335,7 @@ struct VECTOR_SHR_V128 if (i.src2.is_constant) { const auto& shamt = i.src2.constant(); bool all_same = true; - for (size_t n = 0; n < 8 - n; ++n) { + for (size_t n = 0; n < 7; ++n) { if (shamt.u16[n] != shamt.u16[n + 1]) { all_same = false; break; @@ -1418,7 +1418,7 @@ struct VECTOR_SHR_V128 if (i.src2.is_constant) { const auto& shamt = i.src2.constant(); bool all_same = true; - for (size_t n = 0; n < 4 - n; ++n) { + for (size_t n = 0; n < 3; ++n) { if (shamt.u32[n] != shamt.u32[n + 1]) { all_same = false; break; @@ -1636,7 +1636,7 @@ struct VECTOR_SHA_V128 if (i.src2.is_constant) { const auto& shamt = i.src2.constant(); bool all_same = true; - for (size_t n = 0; n < 8 - n; ++n) { + for (size_t n = 0; n < 7; ++n) { if (shamt.u16[n] != shamt.u16[n + 1]) { all_same = false; break; @@ -1711,7 +1711,7 @@ struct VECTOR_SHA_V128 if (i.src2.is_constant) { const auto& shamt = i.src2.constant(); bool all_same = true; - for (size_t n = 0; n < 4 - n; ++n) { + for (size_t n = 0; n < 3; ++n) { if (shamt.u32[n] != shamt.u32[n + 1]) { all_same = false; break; diff --git a/src/xenia/cpu/testing/vector_sha_test.cc b/src/xenia/cpu/testing/vector_sha_test.cc index 187e8b4f2..7179f72f6 100644 --- a/src/xenia/cpu/testing/vector_sha_test.cc +++ b/src/xenia/cpu/testing/vector_sha_test.cc @@ -80,6 +80,27 @@ TEST_CASE("VECTOR_SHA_I8_SAME_CONSTANT", "[instr]") { }); } +TEST_CASE("VECTOR_SHA_I16_CONSTANT_PARTIAL_SAME", "[instr]") { + TestFunction test([](HIRBuilder& b) { + StoreVR(b, 3, + b.VectorSha(LoadVR(b, 4), + b.LoadConstantVec128(vec128s(1, 1, 1, 1, 5, 1, 5, 5)), + INT16_TYPE)); + b.Return(); + }); + test.Run( + [](PPCContext* ctx) { + ctx->v[4] = vec128s(0x8000, 0x8000, 0x8000, 0x8000, 0x8000, 0x8000, + 0x8000, 0x8000); + }, + [](PPCContext* ctx) { + auto result = ctx->v[3]; + // h0-h3,h5: shift 1 → 0xC000; h4,h6,h7: shift 5 → 0xFC00 + REQUIRE(result == vec128s(0xC000, 0xC000, 0xC000, 0xC000, 0xFC00, + 0xC000, 0xFC00, 0xFC00)); + }); +} + TEST_CASE("VECTOR_SHA_I16", "[instr]") { TestFunction test([](HIRBuilder& b) { StoreVR(b, 3, b.VectorSha(LoadVR(b, 4), LoadVR(b, 5), INT16_TYPE)); @@ -119,6 +140,26 @@ TEST_CASE("VECTOR_SHA_I16_CONSTANT", "[instr]") { }); } +TEST_CASE("VECTOR_SHA_I32_CONSTANT_PARTIAL_SAME", "[instr]") { + TestFunction test([](HIRBuilder& b) { + StoreVR( + b, 3, + b.VectorSha(LoadVR(b, 4), b.LoadConstantVec128(vec128i(1, 1, 1, 10)), + INT32_TYPE)); + b.Return(); + }); + test.Run( + [](PPCContext* ctx) { + ctx->v[4] = vec128i(0x80000000, 0x80000000, 0x80000000, 0x80000000); + }, + [](PPCContext* ctx) { + auto result = ctx->v[3]; + // d0-d2: shift 1 → 0xC0000000; d3: shift 10 → 0xFFE00000 + REQUIRE(result == + vec128i(0xC0000000, 0xC0000000, 0xC0000000, 0xFFE00000)); + }); +} + TEST_CASE("VECTOR_SHA_I32", "[instr]") { TestFunction test([](HIRBuilder& b) { StoreVR(b, 3, b.VectorSha(LoadVR(b, 4), LoadVR(b, 5), INT32_TYPE)); diff --git a/src/xenia/cpu/testing/vector_shl_test.cc b/src/xenia/cpu/testing/vector_shl_test.cc index bb821edca..3dd225e11 100644 --- a/src/xenia/cpu/testing/vector_shl_test.cc +++ b/src/xenia/cpu/testing/vector_shl_test.cc @@ -80,6 +80,33 @@ TEST_CASE("VECTOR_SHL_I8_SAME_CONSTANT", "[instr]") { }); } +// Targets the "all_same" detection bug in EmitInt16's constant path. +// The loop condition `n < 8 - n` only checks u16[0..4], missing u16[5..7]. +// vec128s params map to u16[] as: u16[0]=x1, u16[1]=x0, u16[2]=y1, +// u16[3]=y0, u16[4]=z1, u16[5]=z0, u16[6]=w1, u16[7]=w0. +// So params (1,1,1,1,5,1,5,5) → u16[0..4]=1, u16[5..7]=5. +// Buggy code sees all_same=true and shifts everything by 1. +TEST_CASE("VECTOR_SHL_I16_CONSTANT_PARTIAL_SAME", "[instr]") { + TestFunction test([](HIRBuilder& b) { + StoreVR(b, 3, + b.VectorShl(LoadVR(b, 4), + b.LoadConstantVec128(vec128s(1, 1, 1, 1, 5, 1, 5, 5)), + INT16_TYPE)); + b.Return(); + }); + test.Run( + [](PPCContext* ctx) { + ctx->v[4] = vec128s(0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF, + 0xFFFF, 0xFFFF); + }, + [](PPCContext* ctx) { + auto result = ctx->v[3]; + // h0-h3,h5: shift 1 → 0xFFFE; h4,h6,h7: shift 5 → 0xFFE0 + REQUIRE(result == vec128s(0xFFFE, 0xFFFE, 0xFFFE, 0xFFFE, 0xFFE0, + 0xFFFE, 0xFFE0, 0xFFE0)); + }); +} + TEST_CASE("VECTOR_SHL_I16", "[instr]") { TestFunction test([](HIRBuilder& b) { StoreVR(b, 3, b.VectorShl(LoadVR(b, 4), LoadVR(b, 5), INT16_TYPE)); @@ -119,6 +146,30 @@ TEST_CASE("VECTOR_SHL_I16_CONSTANT", "[instr]") { }); } +// Targets the "all_same" detection bug in EmitInt32's constant path. +// The loop condition `n < 4 - n` only checks u32[0..2], missing u32[3]. +// vec128i(1,1,1,10) → u32[0..2]=1, u32[3]=10. +// Buggy code sees all_same=true and shifts everything by 1. +TEST_CASE("VECTOR_SHL_I32_CONSTANT_PARTIAL_SAME", "[instr]") { + TestFunction test([](HIRBuilder& b) { + StoreVR( + b, 3, + b.VectorShl(LoadVR(b, 4), b.LoadConstantVec128(vec128i(1, 1, 1, 10)), + INT32_TYPE)); + b.Return(); + }); + test.Run( + [](PPCContext* ctx) { + ctx->v[4] = vec128i(0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF); + }, + [](PPCContext* ctx) { + auto result = ctx->v[3]; + // d0-d2: shift 1 → 0xFFFFFFFE; d3: shift 10 → 0xFFFFFC00 + REQUIRE(result == + vec128i(0xFFFFFFFE, 0xFFFFFFFE, 0xFFFFFFFE, 0xFFFFFC00)); + }); +} + TEST_CASE("VECTOR_SHL_I32", "[instr]") { TestFunction test([](HIRBuilder& b) { StoreVR(b, 3, b.VectorShl(LoadVR(b, 4), LoadVR(b, 5), INT32_TYPE)); diff --git a/src/xenia/cpu/testing/vector_shr_test.cc b/src/xenia/cpu/testing/vector_shr_test.cc index 000b0234e..51cff1e35 100644 --- a/src/xenia/cpu/testing/vector_shr_test.cc +++ b/src/xenia/cpu/testing/vector_shr_test.cc @@ -80,6 +80,27 @@ TEST_CASE("VECTOR_SHR_I8_SAME_CONSTANT", "[instr]") { }); } +TEST_CASE("VECTOR_SHR_I16_CONSTANT_PARTIAL_SAME", "[instr]") { + TestFunction test([](HIRBuilder& b) { + StoreVR(b, 3, + b.VectorShr(LoadVR(b, 4), + b.LoadConstantVec128(vec128s(1, 1, 1, 1, 5, 1, 5, 5)), + INT16_TYPE)); + b.Return(); + }); + test.Run( + [](PPCContext* ctx) { + ctx->v[4] = vec128s(0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF, 0xFFFF, + 0xFFFF, 0xFFFF); + }, + [](PPCContext* ctx) { + auto result = ctx->v[3]; + // h0-h3,h5: shift 1 → 0x7FFF; h4,h6,h7: shift 5 → 0x07FF + REQUIRE(result == vec128s(0x7FFF, 0x7FFF, 0x7FFF, 0x7FFF, 0x07FF, + 0x7FFF, 0x07FF, 0x07FF)); + }); +} + TEST_CASE("VECTOR_SHR_I16", "[instr]") { TestFunction test([](HIRBuilder& b) { StoreVR(b, 3, b.VectorShr(LoadVR(b, 4), LoadVR(b, 5), INT16_TYPE)); @@ -119,6 +140,26 @@ TEST_CASE("VECTOR_SHR_I16_CONSTANT", "[instr]") { }); } +TEST_CASE("VECTOR_SHR_I32_CONSTANT_PARTIAL_SAME", "[instr]") { + TestFunction test([](HIRBuilder& b) { + StoreVR( + b, 3, + b.VectorShr(LoadVR(b, 4), b.LoadConstantVec128(vec128i(1, 1, 1, 10)), + INT32_TYPE)); + b.Return(); + }); + test.Run( + [](PPCContext* ctx) { + ctx->v[4] = vec128i(0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF, 0xFFFFFFFF); + }, + [](PPCContext* ctx) { + auto result = ctx->v[3]; + // d0-d2: shift 1 → 0x7FFFFFFF; d3: shift 10 → 0x003FFFFF + REQUIRE(result == + vec128i(0x7FFFFFFF, 0x7FFFFFFF, 0x7FFFFFFF, 0x003FFFFF)); + }); +} + TEST_CASE("VECTOR_SHR_I32", "[instr]") { TestFunction test([](HIRBuilder& b) { StoreVR(b, 3, b.VectorShr(LoadVR(b, 4), LoadVR(b, 5), INT32_TYPE));