Commit 81ca3a1d59

David Rubin <daviru007@icloud.com>
2024-07-15 12:37:06
riscv: fix logic bug in `ptr_elem_ptr`
I was doing duplicate work with `elemOffset` multiplying by the abi size and then the `ptr_add` `genBinOp` also multiplying. This led to having writes happening in the wrong place.
master
1 parent cde6956
Changed files (8)
src
arch
riscv64
CodeGen.zig
test
behavior
array.zig
basic.zig
bit_shifting.zig
pointers.zig
slice.zig
union.zig
vector.zig
src/arch/riscv64/CodeGen.zig
@@ -3620,15 +3620,13 @@ fn airPtrElemPtr(func: *Func, inst: Air.Inst.Index) !void {
         defer if (base_ptr_lock) |lock| func.register_manager.unlockReg(lock);
 
         if (elem_ptr_ty.ptrInfo(zcu).flags.vector_index != .none) {
-            break :result if (func.reuseOperand(inst, extra.lhs, 0, base_ptr_mcv))
-                base_ptr_mcv
-            else
-                try func.copyToNewRegister(inst, base_ptr_mcv);
+            // break :result if (func.reuseOperand(inst, extra.lhs, 0, base_ptr_mcv))
+            //     base_ptr_mcv
+            // else
+            //     try func.copyToNewRegister(inst, base_ptr_mcv);
+            @panic("audit");
         }
 
-        const elem_ty = base_ptr_ty.elemType2(zcu);
-        const elem_abi_size = elem_ty.abiSize(pt);
-        const index_ty = func.typeOf(extra.rhs);
         const index_mcv = try func.resolveInst(extra.rhs);
         const index_lock: ?RegisterLock = switch (index_mcv) {
             .register => |reg| func.register_manager.lockRegAssumeUnused(reg),
@@ -3636,10 +3634,6 @@ fn airPtrElemPtr(func: *Func, inst: Air.Inst.Index) !void {
         };
         defer if (index_lock) |lock| func.register_manager.unlockReg(lock);
 
-        const offset_reg = try func.elemOffset(index_ty, index_mcv, elem_abi_size);
-        const offset_reg_lock = func.register_manager.lockRegAssumeUnused(offset_reg);
-        defer func.register_manager.unlockReg(offset_reg_lock);
-
         const result_reg, const result_lock = try func.allocReg(.int);
         defer func.register_manager.unlockReg(result_lock);
 
@@ -3647,7 +3641,7 @@ fn airPtrElemPtr(func: *Func, inst: Air.Inst.Index) !void {
             .ptr_add,
             base_ptr_mcv,
             base_ptr_ty,
-            .{ .register = offset_reg },
+            index_mcv,
             Type.usize,
             result_reg,
         );
test/behavior/array.zig
@@ -22,7 +22,6 @@ test "arrays" {
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
 
     var array: [5]u32 = undefined;
 
@@ -614,7 +613,6 @@ test "type coercion of pointer to anon struct literal to pointer to array" {
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
 
     const S = struct {
         const U = union {
@@ -667,7 +665,6 @@ test "array init of container level array variable" {
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
 
     const S = struct {
         var pair: [2]usize = .{ 1, 2 };
@@ -802,8 +799,6 @@ test "runtime side-effects in comptime-known array init" {
 }
 
 test "slice initialized through reference to anonymous array init provides result types" {
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
-
     var my_u32: u32 = 123;
     var my_u64: u64 = 456;
     _ = .{ &my_u32, &my_u64 };
@@ -817,8 +812,6 @@ test "slice initialized through reference to anonymous array init provides resul
 }
 
 test "sentinel-terminated slice initialized through reference to anonymous array init provides result types" {
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
-
     var my_u32: u32 = 123;
     var my_u64: u64 = 456;
     _ = .{ &my_u32, &my_u64 };
@@ -869,8 +862,6 @@ test "many-item sentinel-terminated pointer initialized through reference to ano
 }
 
 test "pointer to array initialized through reference to anonymous array init provides result types" {
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
-
     var my_u32: u32 = 123;
     var my_u64: u64 = 456;
     _ = .{ &my_u32, &my_u64 };
@@ -884,8 +875,6 @@ test "pointer to array initialized through reference to anonymous array init pro
 }
 
 test "pointer to sentinel-terminated array initialized through reference to anonymous array init provides result types" {
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
-
     var my_u32: u32 = 123;
     var my_u64: u64 = 456;
     _ = .{ &my_u32, &my_u64 };
@@ -912,7 +901,6 @@ test "copied array element doesn't alias source" {
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
 
     var x: [10][10]u32 = undefined;
test/behavior/basic.zig
@@ -620,7 +620,6 @@ var global_ptr = &gdt[0];
 
 test "global constant is loaded with a runtime-known index" {
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
 
     const S = struct {
         fn doTheTest() !void {
test/behavior/bit_shifting.zig
@@ -65,7 +65,6 @@ test "sharded table" {
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest;
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
 
     // realistic 16-way sharding
     try testShardedTable(u32, 4, 8);
test/behavior/pointers.zig
@@ -515,7 +515,6 @@ test "element pointer to slice" {
 test "element pointer arithmetic to slice" {
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
 
     const S = struct {
         fn doTheTest() !void {
test/behavior/slice.zig
@@ -30,7 +30,6 @@ comptime {
 
 test "slicing" {
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
 
     var array: [20]i32 = undefined;
 
@@ -256,7 +255,6 @@ test "C pointer slice access" {
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
 
     var buf: [10]u32 = [1]u32{42} ** 10;
     const c_ptr = @as([*c]const u32, @ptrCast(&buf));
test/behavior/union.zig
@@ -1700,7 +1700,6 @@ test "packed union field pointer has correct alignment" {
 
 test "union with 128 bit integer" {
     if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest;
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
 
     const ValueTag = enum { int, other };
 
@@ -2196,7 +2195,6 @@ test "copied union field doesn't alias source" {
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
 
     const U = union(enum) {
         array: [10]u32,
test/behavior/vector.zig
@@ -395,7 +395,6 @@ test "load vector elements via comptime index" {
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
 
     const S = struct {
         fn doTheTest() !void {
@@ -417,7 +416,6 @@ test "store vector elements via comptime index" {
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
 
     const S = struct {
         fn doTheTest() !void {
@@ -467,7 +465,6 @@ test "store vector elements via runtime index" {
     if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
 
     const S = struct {
         fn doTheTest() !void {
@@ -1511,7 +1508,6 @@ test "vector pointer is indexable" {
     if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO
     if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest; // TODO
-    if (builtin.zig_backend == .stage2_riscv64) return error.SkipZigTest;
 
     const V = @Vector(2, u32);