Merge pull request #17391 from xxxbxxx/load-i4

codegen/llvm: truncate padding bits when loading a non-byte-sized value

src/arch/wasm/CodeGen.zig

@@ -4353,9 +4353,21 @@ fn intcast(func: *CodeGen, operand: WValue, given: Type, wanted: Type) InnerErro
     if (op_bits > 32 and op_bits <= 64 and wanted_bits == 32) {
         try func.emitWValue(operand);
         try func.addTag(.i32_wrap_i64);
+        if (given.isSignedInt(mod) and wanted_bitsize < 32)
+            return func.wrapOperand(.{ .stack = {} }, wanted)
+        else
+            return WValue{ .stack = {} };
     } else if (op_bits == 32 and wanted_bits > 32 and wanted_bits <= 64) {
-        try func.emitWValue(operand);
+        const operand32 = if (given_bitsize < 32 and wanted.isSignedInt(mod))
+            try func.signExtendInt(operand, given)
+        else
+            operand;
+        try func.emitWValue(operand32);
         try func.addTag(if (wanted.isSignedInt(mod)) .i64_extend_i32_s else .i64_extend_i32_u);
+        if (given.isSignedInt(mod) and wanted_bitsize < 64)
+            return func.wrapOperand(.{ .stack = {} }, wanted)
+        else
+            return WValue{ .stack = {} };
     } else if (wanted_bits == 128) {
         // for 128bit integers we store the integer in the virtual stack, rather than a local
         const stack_ptr = try func.allocStack(wanted);
@@ -4381,8 +4393,6 @@ fn intcast(func: *CodeGen, operand: WValue, given: Type, wanted: Type) InnerErro
         }
         return stack_ptr;
     } else return func.load(operand, wanted, 0);
-
-    return WValue{ .stack = {} };
 }
 
 fn airIsNull(func: *CodeGen, inst: Air.Inst.Index, opcode: wasm.Opcode, op_kind: enum { value, ptr }) InnerError!void {

src/codegen/llvm.zig

@@ -6179,7 +6179,6 @@ pub const FuncGen = struct {
                 const elem_alignment = elem_ty.abiAlignment(mod).toLlvm();
                 return self.loadByRef(elem_ptr, elem_ty, elem_alignment, .normal);
             } else {
-                const elem_llvm_ty = try o.lowerType(elem_ty);
                 if (Air.refToIndex(bin_op.lhs)) |lhs_index| {
                     if (self.air.instructions.items(.tag)[lhs_index] == .load) {
                         const load_data = self.air.instructions.items(.data)[lhs_index];
@@ -6201,7 +6200,7 @@ pub const FuncGen = struct {
                                         &indices,
                                         "",
                                     );
-                                    return self.wip.load(.normal, elem_llvm_ty, gep, .default, "");
+                                    return self.loadTruncate(.normal, elem_ty, gep, .default);
                                 },
                                 else => {},
                             }
@@ -6210,7 +6209,7 @@ pub const FuncGen = struct {
                 }
                 const elem_ptr =
                     try self.wip.gep(.inbounds, array_llvm_ty, array_llvm_val, &indices, "");
-                return self.wip.load(.normal, elem_llvm_ty, elem_ptr, .default, "");
+                return self.loadTruncate(.normal, elem_ty, elem_ptr, .default);
             }
         }
 
@@ -6378,13 +6377,12 @@ pub const FuncGen = struct {
                 const payload_index = @intFromBool(layout.tag_align.compare(.gte, layout.payload_align));
                 const field_ptr =
                     try self.wip.gepStruct(union_llvm_ty, struct_llvm_val, payload_index, "");
-                const llvm_field_ty = try o.lowerType(field_ty);
                 const payload_alignment = layout.payload_align.toLlvm();
                 if (isByRef(field_ty, mod)) {
                     if (canElideLoad(self, body_tail)) return field_ptr;
                     return self.loadByRef(field_ptr, field_ty, payload_alignment, .normal);
                 } else {
-                    return self.wip.load(.normal, llvm_field_ty, field_ptr, payload_alignment, "");
+                    return self.loadTruncate(.normal, field_ty, field_ptr, payload_alignment);
                 }
             },
             else => unreachable,
@@ -10219,8 +10217,7 @@ pub const FuncGen = struct {
 
                 return fg.loadByRef(payload_ptr, payload_ty, payload_alignment, .normal);
             }
-            const payload_llvm_ty = try o.lowerType(payload_ty);
-            return fg.wip.load(.normal, payload_llvm_ty, payload_ptr, payload_alignment, "");
+            return fg.loadTruncate(.normal, payload_ty, payload_ptr, payload_alignment);
         }
 
         assert(!isByRef(payload_ty, mod));
@@ -10321,6 +10318,56 @@ pub const FuncGen = struct {
         }
     }
 
+    /// Load a value and, if needed, mask out padding bits for non byte-sized integer values.
+    fn loadTruncate(
+        fg: *FuncGen,
+        access_kind: Builder.MemoryAccessKind,
+        payload_ty: Type,
+        payload_ptr: Builder.Value,
+        payload_alignment: Builder.Alignment,
+    ) !Builder.Value {
+        // from https://llvm.org/docs/LangRef.html#load-instruction :
+        // "When loading a value of a type like i20 with a size that is not an integral number of bytes, the result is undefined if the value was not originally written using a store of the same type. "
+        // => so load the byte aligned value and trunc the unwanted bits.
+
+        const o = fg.dg.object;
+        const mod = o.module;
+        const payload_llvm_ty = try o.lowerType(payload_ty);
+        const abi_size = payload_ty.abiSize(mod);
+
+        // llvm bug workarounds:
+        const workaround_explicit_mask = o.target.cpu.arch == .powerpc and abi_size >= 4;
+        const workaround_disable_truncate = o.target.cpu.arch == .wasm32 and abi_size >= 4;
+
+        if (workaround_disable_truncate) {
+            // see https://github.com/llvm/llvm-project/issues/64222
+            // disable the truncation codepath for larger that 32bits value - with this heuristic, the backend passes the test suite.
+            return try fg.wip.load(access_kind, payload_llvm_ty, payload_ptr, payload_alignment, "");
+        }
+
+        const load_llvm_ty = if (payload_ty.isAbiInt(mod))
+            try o.builder.intType(@intCast(abi_size * 8))
+        else
+            payload_llvm_ty;
+        const loaded = try fg.wip.load(access_kind, load_llvm_ty, payload_ptr, payload_alignment, "");
+        const shifted = if (payload_llvm_ty != load_llvm_ty and o.target.cpu.arch.endian() == .Big)
+            try fg.wip.bin(.lshr, loaded, try o.builder.intValue(
+                load_llvm_ty,
+                (payload_ty.abiSize(mod) - (std.math.divCeil(u64, payload_ty.bitSize(mod), 8) catch unreachable)) * 8,
+            ), "")
+        else
+            loaded;
+
+        const anded = if (workaround_explicit_mask and payload_llvm_ty != load_llvm_ty) blk: {
+            // this is rendundant with llvm.trunc. But without it, llvm17 emits invalid code for powerpc.
+            var mask_val = try o.builder.intConst(payload_llvm_ty, -1);
+            mask_val = try o.builder.castConst(.zext, mask_val, load_llvm_ty);
+            break :blk try fg.wip.bin(.@"and", shifted, mask_val.toValue(), "");
+        } else shifted;
+
+        return fg.wip.conv(.unneeded, anded, payload_llvm_ty, "");
+    }
+
     /// Load a by-ref type by constructing a new alloca and performing a memcpy.
     fn loadByRef(
         fg: *FuncGen,
@@ -10378,7 +10425,7 @@ pub const FuncGen = struct {
             if (isByRef(elem_ty, mod)) {
                 return self.loadByRef(ptr, elem_ty, ptr_alignment, access_kind);
             }
-            return self.wip.load(access_kind, try o.lowerType(elem_ty), ptr, ptr_alignment, "");
+            return self.loadTruncate(access_kind, elem_ty, ptr, ptr_alignment);
         }
 
         const containing_int_ty = try o.builder.intType(@intCast(info.packed_offset.host_size * 8));

test/behavior/cast_int.zig

@@ -1,6 +1,7 @@
 const builtin = @import("builtin");
 const std = @import("std");
 const expect = std.testing.expect;
+const expectEqual = std.testing.expectEqual;
 const maxInt = std.math.maxInt;
 
 test "@intCast i32 to u7" {
@@ -28,3 +29,197 @@ test "coerce i8 to i32 and @intCast back" {
     var y2: i8 = -5;
     try expect(y2 == @as(i8, @intCast(x2)));
 }
+
+test "coerce non byte-sized integers accross 32bits boundary" {
+    {
+        var v: u21 = 6417;
+        const a: u32 = v;
+        const b: u64 = v;
+        const c: u64 = a;
+        var w: u64 = 0x1234567812345678;
+        const d: u21 = @truncate(w);
+        const e: u60 = d;
+        try expectEqual(@as(u32, 6417), a);
+        try expectEqual(@as(u64, 6417), b);
+        try expectEqual(@as(u64, 6417), c);
+        try expectEqual(@as(u21, 0x145678), d);
+        try expectEqual(@as(u60, 0x145678), e);
+    }
+
+    {
+        var v: u10 = 234;
+        const a: u32 = v;
+        const b: u64 = v;
+        const c: u64 = a;
+        var w: u64 = 0x1234567812345678;
+        const d: u10 = @truncate(w);
+        const e: u60 = d;
+        try expectEqual(@as(u32, 234), a);
+        try expectEqual(@as(u64, 234), b);
+        try expectEqual(@as(u64, 234), c);
+        try expectEqual(@as(u21, 0x278), d);
+        try expectEqual(@as(u60, 0x278), e);
+    }
+    {
+        var v: u7 = 11;
+        const a: u32 = v;
+        const b: u64 = v;
+        const c: u64 = a;
+        var w: u64 = 0x1234567812345678;
+        const d: u7 = @truncate(w);
+        const e: u60 = d;
+        try expectEqual(@as(u32, 11), a);
+        try expectEqual(@as(u64, 11), b);
+        try expectEqual(@as(u64, 11), c);
+        try expectEqual(@as(u21, 0x78), d);
+        try expectEqual(@as(u60, 0x78), e);
+    }
+
+    {
+        var v: i21 = -6417;
+        const a: i32 = v;
+        const b: i64 = v;
+        const c: i64 = a;
+        var w: i64 = -12345;
+        const d: i21 = @intCast(w);
+        const e: i60 = d;
+        try expectEqual(@as(i32, -6417), a);
+        try expectEqual(@as(i64, -6417), b);
+        try expectEqual(@as(i64, -6417), c);
+        try expectEqual(@as(i21, -12345), d);
+        try expectEqual(@as(i60, -12345), e);
+    }
+
+    {
+        var v: i10 = -234;
+        const a: i32 = v;
+        const b: i64 = v;
+        const c: i64 = a;
+        var w: i64 = -456;
+        const d: i10 = @intCast(w);
+        const e: i60 = d;
+        try expectEqual(@as(i32, -234), a);
+        try expectEqual(@as(i64, -234), b);
+        try expectEqual(@as(i64, -234), c);
+        try expectEqual(@as(i10, -456), d);
+        try expectEqual(@as(i60, -456), e);
+    }
+    {
+        var v: i7 = -11;
+        const a: i32 = v;
+        const b: i64 = v;
+        const c: i64 = a;
+        var w: i64 = -42;
+        const d: i7 = @intCast(w);
+        const e: i60 = d;
+        try expectEqual(@as(i32, -11), a);
+        try expectEqual(@as(i64, -11), b);
+        try expectEqual(@as(i64, -11), c);
+        try expectEqual(@as(i7, -42), d);
+        try expectEqual(@as(i60, -42), e);
+    }
+}
+
+const Piece = packed struct {
+    color: Color,
+    type: Type,
+
+    const Type = enum { KING, QUEEN, BISHOP, KNIGHT, ROOK, PAWN };
+    const Color = enum { WHITE, BLACK };
+
+    fn charToPiece(c: u8) !@This() {
+        return .{
+            .type = try charToPieceType(c),
+            .color = if (std.ascii.isUpper(c)) Color.WHITE else Color.BLACK,
+        };
+    }
+
+    fn charToPieceType(c: u8) !Type {
+        return switch (std.ascii.toLower(c)) {
+            'p' => .PAWN,
+            'k' => .KING,
+            'q' => .QUEEN,
+            'b' => .BISHOP,
+            'n' => .KNIGHT,
+            'r' => .ROOK,
+            else => error.UnexpectedCharError,
+        };
+    }
+};
+
+test "load non byte-sized optional value" {
+    // Originally reported at https://github.com/ziglang/zig/issues/14200
+    // note: this bug is triggered by the == operator, expectEqual will hide it
+    var opt: ?Piece = try Piece.charToPiece('p');
+    try expect(opt.?.type == .PAWN);
+    try expect(opt.?.color == .BLACK);
+
+    var p: Piece = undefined;
+    @as(*u8, @ptrCast(&p)).* = 0b11111011;
+    try expect(p.type == .PAWN);
+    try expect(p.color == .BLACK);
+}
+
+test "load non byte-sized value in struct" {
+    if (builtin.cpu.arch.endian() != .Little) return error.SkipZigTest; // packed struct TODO
+
+    // note: this bug is triggered by the == operator, expectEqual will hide it
+    // using ptrCast not to depend on unitialised memory state
+
+    var struct0: struct {
+        p: Piece,
+        int: u8,
+    } = undefined;
+    @as(*u8, @ptrCast(&struct0.p)).* = 0b11111011;
+    try expect(struct0.p.type == .PAWN);
+    try expect(struct0.p.color == .BLACK);
+
+    var struct1: packed struct {
+        p0: Piece,
+        p1: Piece,
+        pad: u1,
+        p2: Piece,
+    } = undefined;
+    @as(*u8, @ptrCast(&struct1.p0)).* = 0b11111011;
+    struct1.p1 = try Piece.charToPiece('p');
+    struct1.p2 = try Piece.charToPiece('p');
+    try expect(struct1.p0.type == .PAWN);
+    try expect(struct1.p0.color == .BLACK);
+    try expect(struct1.p1.type == .PAWN);
+    try expect(struct1.p1.color == .BLACK);
+    try expect(struct1.p2.type == .PAWN);
+    try expect(struct1.p2.color == .BLACK);
+}
+
+test "load non byte-sized value in union" {
+    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;
+    if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest;
+    if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest;
+    if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest;
+
+    // note: this bug is triggered by the == operator, expectEqual will hide it
+    // using ptrCast not to depend on unitialised memory state
+
+    var union0: packed union {
+        p: Piece,
+        int: u8,
+    } = .{ .int = 0 };
+    union0.int = 0b11111011;
+    try expect(union0.p.type == .PAWN);
+    try expect(union0.p.color == .BLACK);
+
+    var union1: union {
+        p: Piece,
+        int: u8,
+    } = .{ .p = .{ .color = .WHITE, .type = .KING } };
+    @as(*u8, @ptrCast(&union1.p)).* = 0b11111011;
+    try expect(union1.p.type == .PAWN);
+    try expect(union1.p.color == .BLACK);
+
+    var pieces: [3]Piece = undefined;
+    @as(*u8, @ptrCast(&pieces[1])).* = 0b11111011;
+    try expect(pieces[1].type == .PAWN);
+    try expect(pieces[1].color == .BLACK);
+}

test/behavior/packed-struct.zig

@@ -991,7 +991,7 @@ test "bitcast back and forth" {
 
 test "field access of packed struct smaller than its abi size inside struct initialized with rls" {
     // Originally reported at https://github.com/ziglang/zig/issues/14200
-    if (builtin.zig_backend == .stage2_llvm and builtin.cpu.arch == .arm) return error.SkipZigTest;
+
     const S = struct {
         ps: packed struct { x: i2, y: i2 },
 
@@ -1036,3 +1036,32 @@ test "modify nested packed struct aligned field" {
     try std.testing.expectEqual(@as(u8, 1), opts.pretty_print.indent);
     try std.testing.expect(!opts.baz);
 }
+
+test "assigning packed struct inside another packed struct" {
+    // Originally reported at https://github.com/ziglang/zig/issues/9674
+
+    const S = struct {
+        const Inner = packed struct {
+            bits: u3,
+            more_bits: u6,
+        };
+
+        const Outer = packed struct {
+            padding: u5,
+            inner: Inner,
+        };
+        fn t(inner: Inner) void {
+            r.inner = inner;
+        }
+
+        var mem: Outer = undefined;
+        var r: *volatile Outer = &mem;
+    };
+
+    const val = S.Inner{ .bits = 1, .more_bits = 11 };
+    S.mem.padding = 0;
+    S.t(val);
+
+    try expectEqual(val, S.mem.inner);
+    try expect(S.mem.padding == 0);
+}

test/behavior/packed-union.zig

@@ -85,3 +85,31 @@ test "flags in packed union at offset" {
     try expectEqual(false, test_bits.adv_flags.adv.flags.enable_1);
     try expectEqual(false, test_bits.adv_flags.adv.flags.enable_2);
 }
+
+test "packed union in packed struct" {
+    // Originally reported at https://github.com/ziglang/zig/issues/16581
+
+    const ReadRequest = packed struct { key: i32 };
+    const RequestType = enum {
+        read,
+        insert,
+    };
+    const RequestUnion = packed union {
+        read: ReadRequest,
+    };
+
+    const Request = packed struct {
+        active_type: RequestType,
+        request: RequestUnion,
+        const Self = @This();
+
+        fn init(read: ReadRequest) Self {
+            return .{
+                .active_type = .read,
+                .request = RequestUnion{ .read = read },
+            };
+        }
+    };
+
+    try std.testing.expectEqual(RequestType.read, Request.init(.{ .key = 3 }).active_type);
+}