remove elem_val and field_val ZIR instructions; forbid runtime vector indexes

[andrewrk]

fixes #22906
fixes #13938
fixes #25111

related: some other issue that @mlugg mentioned regarding runtime vector issues

Upgrade Guide

for -> inline for when iterating over vector elements.

Merge Checklist

• fix the failures
• add coverage for the aliasing being gone
• there are a few more related issues to find and close

[andrewrk]

Soliciting help from @jacobly0:

Basically the result of @addWithOverflow and friends are taking a detour through being stored in a local and then accessed via loading from the local.

stage4/bin/zig test ../test/behavior.zig

# Begin Function AIR: math.sub__anon_26719:
# Total AIR+Liveness bytes: 556B
# AIR Instructions:         28 (252B)
# AIR Extra Data:           36 (144B)
# Liveness tomb_bits:       16B
# Liveness Extra Data:      4 (16B)
# Liveness special table:   2 (16B)
  %0 = arg(u31, 1)
  %1 = arg(u31, 2)
  %2!= save_err_return_trace_index()
  %3!= dbg_stmt(3:5)
  %6 = sub_with_overflow(struct { u31, u1 }, %0!, %1!)
  %7 = alloc(*struct { u31, u1 })
  %8!= store_safe(%7, %6)
  %9 = bitcast(*const struct { u31, u1 }, %7!)
  %10!= dbg_var_val(%6!, "ov")
  %11!= dbg_stmt(4:9)
  %12!= block(void, {
    %13!= dbg_stmt(4:11)
    %14 = struct_field_ptr_index_1(*const u1, %9)
    %15 = load(u1, %14!)
    %16 = cmp_neq(%15!, @.zero_u1)
    %21!= cond_br(%16!, poi {
      %9!
      %17!= dbg_stmt(4:21)
      %18!= call_never_tail(<noinline fn () void, (function 'returnError')>, [])
      %19!= ret_safe(<error{Overflow}!u31, error.Overflow>)
    }, poi {
      %20!= br(%12, @.void_value)
    })
  })
  %22!= dbg_stmt(5:14)
  %23 = struct_field_ptr_index_0(*const u31, %9!)
  %24 = load(u31, %23!)
  %25!= dbg_stmt(5:5)
  %26 = wrap_errunion_payload(error{Overflow}!u31, %24!)
  %27!= ret_safe(%26!)
# End Function AIR: math.sub__anon_26719

thread 1854526 panic: storeRegs: struct { u31, u1 }

/home/andy/dev/zig/src/arch/x86_64/CodeGen.zig:186921:40: 0xb459beb in storeRegs (main.zig)
                else => std.debug.panic("{s}: {f}\n", .{ @src().fn_name, src_ty.fmt(cg.pt) }),
                                       ^
/home/andy/dev/zig/src/arch/x86_64/CodeGen.zig:186713:48: 0xadf0105 in store (main.zig)
                => |val_regs| try ptr.storeRegs(val_ty, &val_regs, cg),
                                               ^
/home/andy/dev/zig/src/arch/x86_64/CodeGen.zig:86810:33: 0xa72e10a in genBody (main.zig)
                try ops[0].store(&ops[1], .{
                                ^
/home/andy/dev/zig/src/arch/x86_64/CodeGen.zig:2268:19: 0x9ce312a in genMainBody (main.zig)
    try cg.genBody(main_body[air_arg_count..]);
                  ^
/home/andy/dev/zig/src/arch/x86_64/CodeGen.zig:2064:29: 0x9446031 in gen (main.zig)
        try self.genMainBody(zir, func_zir_inst, comptime_args, air_arg_count);
                            ^
/home/andy/dev/zig/src/arch/x86_64/CodeGen.zig:991:17: 0x8984738 in generate (main.zig)
    function.gen(&file.zir.?, func_zir.inst, func.comptime_args, call_info.air_arg_count) catch |err| switch (err) {
                ^
/home/andy/dev/zig/src/codegen.zig:164:45: 0x7e2acdb in generateFunction (main.zig)
            const mir = try CodeGen.generate(lf, pt, src_loc, func_index, air, liveness);
                                            ^
/home/andy/dev/zig/src/Zcu/PerThread.zig:4501:36: 0x7758121 in runCodegenInner (main.zig)
    return codegen.generateFunction(lf, pt, zcu.navSrcLoc(nav), func_index, air, &liveness) catch |err| switch (err) {
                                   ^
/home/andy/dev/zig/src/Zcu/PerThread.zig:4379:46: 0x713242d in runCodegen (main.zig)
    const success: bool = if (runCodegenInner(pt, func_index, air)) |mir| success: {
                                             ^
/home/andy/dev/zig/src/Compilation.zig:5838:18: 0x7743011 in workerZcuCodegen (main.zig)
    pt.runCodegen(func_index, &air, out);
                 ^
/home/andy/dev/zig/lib/std/Thread/Pool.zig:180:50: 0x774344c in runFn (std.zig)
            @call(.auto, func, .{id.?} ++ closure.arguments);
                                                 ^
/home/andy/dev/zig/lib/std/Thread/Pool.zig:293:27: 0x705b84d in worker (std.zig)
            runnable.runFn(runnable, id);
                          ^
/home/andy/dev/zig/lib/std/Thread.zig:511:13: 0x6e103e0 in callFn__anon_186686 (std.zig)
            @call(.auto, f, args);
            ^
/home/andy/dev/zig/lib/std/Thread.zig:783:30: 0x6c3c679 in entryFn (std.zig)
                return callFn(f, args_ptr.*);
                             ^
???:?:?: 0x7ff0b8497ea2 in ??? (libc.so.6)
Unwind information for `libc.so.6:0x7ff0b8497ea2` was not available, trace may be incomplete

[jacobly0]

--- a/src/arch/x86_64/CodeGen.zig
+++ b/src/arch/x86_64/CodeGen.zig
@@ -186936,7 +186936,7 @@ const Temp = struct {
                         break :part_ty .usize;
                     },
                 },
-                .struct_type => {
+                .struct_type, .tuple_type => {
                     assert(src_regs.len - part_index == std.math.divCeil(u32, src_abi_size, 8) catch unreachable);
                     break :part_ty .u64;
                 },

[andrewrk]

Ah of course thank you!

[andrewrk]

It trips that assert:

/home/andy/dev/zig/src/arch/x86_64/CodeGen.zig:186940:27: 0xb461537 in storeRegs (main.zig)
                    assert(src_regs.len - part_index == std.math.divCeil(u32, src_abi_size, 8) catch unreachable);
                          ^

[jacobly0]

This kind of works

--- a/src/arch/x86_64/CodeGen.zig
+++ b/src/arch/x86_64/CodeGen.zig
@@ -186940,6 +186940,10 @@ const Temp = struct {
                     assert(src_regs.len - part_index == std.math.divCeil(u32, src_abi_size, 8) catch unreachable);
                     break :part_ty .u64;
                 },
+                .tuple_type => |tuple_type| {
+                    assert(tuple_type.types.len == src_regs.len);
+                    break :part_ty .fromInterned(tuple_type.types.get(ip)[part_index]);
+                },
             };
             const part_size: u31 = @intCast(part_ty.abiSize(zcu));
             const src_rc = src_reg.class();

[andrewrk]

Reduction for analysis:

const std = @import("std");

pub const panic = std.debug.no_panic;

export fn entry() void {
    comptime var map: [10]usize = @splat(0);
    map[0] = 0;

    var runtime_index: usize = 0;
    _ = &runtime_index;
    const set_index = map[runtime_index];
    _ = set_index;
}

$ stage4/bin/zig build-obj test.zig
test.zig:11:26: error: runtime value contains reference to comptime var
    const set_index = map[runtime_index];
                      ~~~^~~~~~~~~~~~~~~
test.zig:11:26: note: comptime var pointers are not available at runtime
test.zig:6:35: note: 'runtime_value' points to comptime var declared here
    comptime var map: [10]usize = @splat(0);
                                  ^~~~~~~~~
referenced by:
    entry: test.zig:5:1
    root: /home/andy/dev/zig/lib/std/start.zig:3:22
    3 reference(s) hidden; use '-freference-trace=5' to see all references

I argue this is a correct error. Before it was relying on map being copied for each access with a runtime-known index. Now, if the programmer wants equivalent semantics, they must explicitly make such a copy:

    const copy = map;
    const set_index = copy[runtime_index];

Or, more likely, they will realize this is creating bloat in the binary and instead create only 1 copy of the runtime data and access it several times.

In summary it exposes an invisible copy.

[andrewrk]

superseded by #25154