From c9249c6c96375544d27a5c5c463b1a9f86b5e964 Mon Sep 17 00:00:00 2001 From: MarcelineVQ Date: Sun, 15 Mar 2026 12:27:59 -0700 Subject: [PATCH] bench: fix 3 correctness bugs found by benchmarker mulMat3x4 / mulMat3x4InPlace: translation row had A*B operands swapped. Original computes A_translation * B_rotation + B_translation, but our code was doing A_rotation * B_translation + A_translation. Verified by tracing x87 disassembly: first element loads A[9]*B[col] pattern. Fixed in both silicon_sse.zig and silicon.zig. packParticleColor: x87 rounds 127.5 to 128 (round-to-nearest), but SSE @intFromFloat truncates to 127. Added @round() before @intFromFloat. 42/42 benchmarks now pass correctness. 0 MISMATCHes. --- src/bench/main.zig | 21 +++++++++++++++++++++ src/silicon/silicon.zig | 4 ++-- src/silicon/silicon_sse.zig | 10 +++++----- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/bench/main.zig b/src/bench/main.zig index 682a136..042c883 100644 --- a/src/bench/main.zig +++ b/src/bench/main.zig @@ -596,6 +596,14 @@ pub fn main() void { _ = of(a(&ro), a(&ma), a(&mb)); _ = si_mulMat3x4(a(&rs), a(&ma), a(&mb)); const ok = cmpSlice(&ro, &rs); + if (!ok) { + print(" mulMat3x4 MISMATCH detail:\n", .{}); + for (0..12) |i| { + if (!compareF32(ro[i], rs[i])) { + print(" [{d}] orig={d} sse={d}\n", .{ i, @as(i32, @intFromFloat(ro[i] * 1000)), @as(i32, @intFromFloat(rs[i] * 1000)) }); + } + } + } var t = rdtsc(); for (0..ITERS) |_| { _ = of(a(&ro), a(&ma), a(&mb)); } t = rdtsc() - t; var s = rdtsc(); for (0..ITERS) |_| { _ = si_mulMat3x4(a(&rs), a(&ma), a(&mb)); } s = rdtsc() - s; report("mulMat3x4", t, s, ok); @@ -641,6 +649,14 @@ pub fn main() void { _ = of(a(&mo2), a(&mb2)); _ = si_mulMat3x4InPlace(a(&ms2), a(&mb2)); const ok = cmpSlice(&mo2, &ms2); + if (!ok) { + print(" mulMat3x4InPlace MISMATCH detail:\n", .{}); + for (0..12) |i| { + if (!compareF32(mo2[i], ms2[i])) { + print(" [{d}] orig={d} sse={d}\n", .{ i, @as(i32, @intFromFloat(mo2[i] * 1000)), @as(i32, @intFromFloat(ms2[i] * 1000)) }); + } + } + } var t = rdtsc(); for (0..ITERS) |_| { mo2 = tmpl2; _ = of(a(&mo2), a(&mb2)); } t = rdtsc() - t; var s = rdtsc(); for (0..ITERS) |_| { ms2 = tmpl2; _ = si_mulMat3x4InPlace(a(&ms2), a(&mb2)); } s = rdtsc() - s; report("mulMat3x4InPlace", t, s, ok); @@ -761,6 +777,11 @@ pub fn main() void { const out_o = @as(*align(1) const u32, @ptrCast(&obj_o[0x12C])).*; const out_s = @as(*align(1) const u32, @ptrCast(&obj_s[0x12C])).*; const ok = out_o == out_s; + if (!ok) { + print(" packParticleColor MISMATCH: orig=0x{x} sse=0x{x}\n", .{ out_o, out_s }); + print(" orig bytes: [{x} {x} {x} {x}]\n", .{ obj_o[0x12C], obj_o[0x12D], obj_o[0x12E], obj_o[0x12F] }); + print(" sse bytes: [{x} {x} {x} {x}]\n", .{ obj_s[0x12C], obj_s[0x12D], obj_s[0x12E], obj_s[0x12F] }); + } var t = rdtsc(); for (0..ITERS) |_| { of(a(&obj_o), 0, rb, gb, bb); } t = rdtsc() - t; var s = rdtsc(); for (0..ITERS) |_| { si_packParticleColor(a(&obj_s), rb, gb, bb); } s = rdtsc() - s; report("packParticleColor", t, s, ok); diff --git a/src/silicon/silicon.zig b/src/silicon/silicon.zig index 2eddb62..c252f9d 100644 --- a/src/silicon/silicon.zig +++ b/src/silicon/silicon.zig @@ -1166,7 +1166,7 @@ fn sseMulMat3x4(out: u32, a_ptr: u32, b_ptr: u32) callconv(FC) u32 { } // Translation row (indices 9-11): same multiply + add B's translation inline for (0..3) |col| { - dst[9 + col] = a[col] * b[9] + a[col + 3] * b[10] + a[col + 6] * b[11] + a[9 + col]; + dst[9 + col] = a[9] * b[col] + a[10] * b[col + 3] + a[11] * b[col + 6] + b[9 + col]; } return out; } @@ -1427,7 +1427,7 @@ fn sseMulMat3x4InPlace(mat_a: u32, mat_b: u32) callconv(TC) u32 { } } inline for (0..3) |col| { - tmp[9 + col] = a[col] * b[9] + a[col + 3] * b[10] + a[col + 6] * b[11] + a[9 + col]; + tmp[9 + col] = a[9] * b[col] + a[10] * b[col + 3] + a[11] * b[col + 6] + b[9 + col]; } // Copy back inline for (0..12) |i| { diff --git a/src/silicon/silicon_sse.zig b/src/silicon/silicon_sse.zig index faf92a6..4bee441 100644 --- a/src/silicon/silicon_sse.zig +++ b/src/silicon/silicon_sse.zig @@ -32,7 +32,7 @@ export fn si_mulMat3x4(out: u32, a_ptr: u32, b_ptr: u32) u32 { } } inline for (0..3) |col| { - dst[9 + col] = a[col] * b[9] + a[col + 3] * b[10] + a[col + 6] * b[11] + a[9 + col]; + dst[9 + col] = a[9] * b[col] + a[10] * b[col + 3] + a[11] * b[col + 6] + b[9 + col]; } return out; } @@ -270,7 +270,7 @@ export fn si_mulMat3x4InPlace(mat_a: u32, mat_b: u32) u32 { } } inline for (0..3) |col| { - tmp[9 + col] = a[col] * b[9] + a[col + 3] * b[10] + a[col + 6] * b[11] + a[9 + col]; + tmp[9 + col] = a[9] * b[col] + a[10] * b[col + 3] + a[11] * b[col + 6] + b[9 + col]; } inline for (0..12) |i| { a[i] = tmp[i]; } return mat_a; @@ -321,9 +321,9 @@ export fn si_packParticleColor(obj: u32, r_bits: u32, g_bits: u32, b_bits: u32) const r: f32 = @bitCast(r_bits); const g: f32 = @bitCast(g_bits); const b: f32 = @bitCast(b_bits); - const rb: u8 = @intFromFloat(@min(@max(r * 255.0, 0.0), 255.0)); - const gb: u8 = @intFromFloat(@min(@max(g * 255.0, 0.0), 255.0)); - const bb: u8 = @intFromFloat(@min(@max(b * 255.0, 0.0), 255.0)); + const rb: u8 = @intFromFloat(@round(@min(@max(r * 255.0, 0.0), 255.0))); + const gb: u8 = @intFromFloat(@round(@min(@max(g * 255.0, 0.0), 255.0))); + const bb: u8 = @intFromFloat(@round(@min(@max(b * 255.0, 0.0), 255.0))); out.* = @as(u32, alpha) << 24 | @as(u32, rb) << 16 | @as(u32, gb) << 8 | @as(u32, bb); }