fabi/Xenia-Canary
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[CPU] Fix incorrect all_same detection in constant vector shift paths

Browse Source 
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.
This commit is contained in:
Herman S.
2026-02-12 09:31:54 +09:00
parent 10e8224a63
commit bb70e5c651
4 changed files with 139 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
src/xenia/cpu/backend/x64/x64_seq_vector.cc
View File

@@ -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;

41
src/xenia/cpu/testing/vector_sha_test.cc
View File

@@ -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));

51
src/xenia/cpu/testing/vector_shl_test.cc
View File

@@ -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));

41
src/xenia/cpu/testing/vector_shr_test.cc
View File

@@ -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));