Skip to content

Commit aaf4618

Browse files
andrewrkandrewrk
authored
Merge pull request #17391 from xxxbxxx/load-i4
codegen/llvm: truncate padding bits when loading a non-byte-sized value
2 parents fa08e49 + 703fe0f commit aaf4618

File tree

5 files changed

+321
-12
lines changed

5 files changed

+321
-12
lines changed

‎src/arch/wasm/CodeGen.zig

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4353,9 +4353,21 @@ fn intcast(func: *CodeGen, operand: WValue, given: Type, wanted: Type) InnerErro
43534353
if (op_bits > 32 and op_bits <= 64 and wanted_bits == 32) {
43544354
try func.emitWValue(operand);
43554355
try func.addTag(.i32_wrap_i64);
4356+
if (given.isSignedInt(mod) and wanted_bitsize < 32)
4357+
return func.wrapOperand(.{ .stack = {} }, wanted)
4358+
else
4359+
return WValue{ .stack = {} };
43564360
} else if (op_bits == 32 and wanted_bits > 32 and wanted_bits <= 64) {
4357-
try func.emitWValue(operand);
4361+
const operand32 = if (given_bitsize < 32 and wanted.isSignedInt(mod))
4362+
try func.signExtendInt(operand, given)
4363+
else
4364+
operand;
4365+
try func.emitWValue(operand32);
43584366
try func.addTag(if (wanted.isSignedInt(mod)) .i64_extend_i32_s else .i64_extend_i32_u);
4367+
if (given.isSignedInt(mod) and wanted_bitsize < 64)
4368+
return func.wrapOperand(.{ .stack = {} }, wanted)
4369+
else
4370+
return WValue{ .stack = {} };
43594371
} else if (wanted_bits == 128) {
43604372
// for 128bit integers we store the integer in the virtual stack, rather than a local
43614373
const stack_ptr = try func.allocStack(wanted);
@@ -4381,8 +4393,6 @@ fn intcast(func: *CodeGen, operand: WValue, given: Type, wanted: Type) InnerErro
43814393
}
43824394
return stack_ptr;
43834395
} else return func.load(operand, wanted, 0);
4384-
4385-
return WValue{ .stack = {} };
43864396
}
43874397

43884398
fn airIsNull(func: *CodeGen, inst: Air.Inst.Index, opcode: wasm.Opcode, op_kind: enum { value, ptr }) InnerError!void {

‎src/codegen/llvm.zig

Lines changed: 55 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6179,7 +6179,6 @@ pub const FuncGen = struct {
61796179
const elem_alignment = elem_ty.abiAlignment(mod).toLlvm();
61806180
return self.loadByRef(elem_ptr, elem_ty, elem_alignment, .normal);
61816181
} else {
6182-
const elem_llvm_ty = try o.lowerType(elem_ty);
61836182
if (Air.refToIndex(bin_op.lhs)) |lhs_index| {
61846183
if (self.air.instructions.items(.tag)[lhs_index] == .load) {
61856184
const load_data = self.air.instructions.items(.data)[lhs_index];
@@ -6201,7 +6200,7 @@ pub const FuncGen = struct {
62016200
&indices,
62026201
"",
62036202
);
6204-
return self.wip.load(.normal, elem_llvm_ty, gep, .default, "");
6203+
return self.loadTruncate(.normal, elem_ty, gep, .default);
62056204
},
62066205
else => {},
62076206
}
@@ -6210,7 +6209,7 @@ pub const FuncGen = struct {
62106209
}
62116210
const elem_ptr =
62126211
try self.wip.gep(.inbounds, array_llvm_ty, array_llvm_val, &indices, "");
6213-
return self.wip.load(.normal, elem_llvm_ty, elem_ptr, .default, "");
6212+
return self.loadTruncate(.normal, elem_ty, elem_ptr, .default);
62146213
}
62156214
}
62166215

@@ -6378,13 +6377,12 @@ pub const FuncGen = struct {
63786377
const payload_index = @intFromBool(layout.tag_align.compare(.gte, layout.payload_align));
63796378
const field_ptr =
63806379
try self.wip.gepStruct(union_llvm_ty, struct_llvm_val, payload_index, "");
6381-
const llvm_field_ty = try o.lowerType(field_ty);
63826380
const payload_alignment = layout.payload_align.toLlvm();
63836381
if (isByRef(field_ty, mod)) {
63846382
if (canElideLoad(self, body_tail)) return field_ptr;
63856383
return self.loadByRef(field_ptr, field_ty, payload_alignment, .normal);
63866384
} else {
6387-
return self.wip.load(.normal, llvm_field_ty, field_ptr, payload_alignment, "");
6385+
return self.loadTruncate(.normal, field_ty, field_ptr, payload_alignment);
63886386
}
63896387
},
63906388
else => unreachable,
@@ -10219,8 +10217,7 @@ pub const FuncGen = struct {
1021910217

1022010218
return fg.loadByRef(payload_ptr, payload_ty, payload_alignment, .normal);
1022110219
}
10222-
const payload_llvm_ty = try o.lowerType(payload_ty);
10223-
return fg.wip.load(.normal, payload_llvm_ty, payload_ptr, payload_alignment, "");
10220+
return fg.loadTruncate(.normal, payload_ty, payload_ptr, payload_alignment);
1022410221
}
1022510222

1022610223
assert(!isByRef(payload_ty, mod));
@@ -10321,6 +10318,56 @@ pub const FuncGen = struct {
1032110318
}
1032210319
}
1032310320

10321+
/// Load a value and, if needed, mask out padding bits for non byte-sized integer values.
10322+
fn loadTruncate(
10323+
fg: *FuncGen,
10324+
access_kind: Builder.MemoryAccessKind,
10325+
payload_ty: Type,
10326+
payload_ptr: Builder.Value,
10327+
payload_alignment: Builder.Alignment,
10328+
) !Builder.Value {
10329+
// from https://llvm.org/docs/LangRef.html#load-instruction :
10330+
// "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. "
10331+
// => so load the byte aligned value and trunc the unwanted bits.
10332+
10333+
const o = fg.dg.object;
10334+
const mod = o.module;
10335+
const payload_llvm_ty = try o.lowerType(payload_ty);
10336+
const abi_size = payload_ty.abiSize(mod);
10337+
10338+
// llvm bug workarounds:
10339+
const workaround_explicit_mask = o.target.cpu.arch == .powerpc and abi_size >= 4;
10340+
const workaround_disable_truncate = o.target.cpu.arch == .wasm32 and abi_size >= 4;
10341+
10342+
if (workaround_disable_truncate) {
10343+
// see https://github.com/llvm/llvm-project/issues/64222
10344+
// disable the truncation codepath for larger that 32bits value - with this heuristic, the backend passes the test suite.
10345+
return try fg.wip.load(access_kind, payload_llvm_ty, payload_ptr, payload_alignment, "");
10346+
}
10347+
10348+
const load_llvm_ty = if (payload_ty.isAbiInt(mod))
10349+
try o.builder.intType(@intCast(abi_size * 8))
10350+
else
10351+
payload_llvm_ty;
10352+
const loaded = try fg.wip.load(access_kind, load_llvm_ty, payload_ptr, payload_alignment, "");
10353+
const shifted = if (payload_llvm_ty != load_llvm_ty and o.target.cpu.arch.endian() == .Big)
10354+
try fg.wip.bin(.lshr, loaded, try o.builder.intValue(
10355+
load_llvm_ty,
10356+
(payload_ty.abiSize(mod) - (std.math.divCeil(u64, payload_ty.bitSize(mod), 8) catch unreachable)) * 8,
10357+
), "")
10358+
else
10359+
loaded;
10360+
10361+
const anded = if (workaround_explicit_mask and payload_llvm_ty != load_llvm_ty) blk: {
10362+
// this is rendundant with llvm.trunc. But without it, llvm17 emits invalid code for powerpc.
10363+
var mask_val = try o.builder.intConst(payload_llvm_ty, -1);
10364+
mask_val = try o.builder.castConst(.zext, mask_val, load_llvm_ty);
10365+
break :blk try fg.wip.bin(.@"and", shifted, mask_val.toValue(), "");
10366+
} else shifted;
10367+
10368+
return fg.wip.conv(.unneeded, anded, payload_llvm_ty, "");
10369+
}
10370+
1032410371
/// Load a by-ref type by constructing a new alloca and performing a memcpy.
1032510372
fn loadByRef(
1032610373
fg: *FuncGen,
@@ -10378,7 +10425,7 @@ pub const FuncGen = struct {
1037810425
if (isByRef(elem_ty, mod)) {
1037910426
return self.loadByRef(ptr, elem_ty, ptr_alignment, access_kind);
1038010427
}
10381-
return self.wip.load(access_kind, try o.lowerType(elem_ty), ptr, ptr_alignment, "");
10428+
return self.loadTruncate(access_kind, elem_ty, ptr, ptr_alignment);
1038210429
}
1038310430

1038410431
const containing_int_ty = try o.builder.intType(@intCast(info.packed_offset.host_size * 8));

‎test/behavior/cast_int.zig

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
const builtin = @import("builtin");
22
const std = @import("std");
33
const expect = std.testing.expect;
4+
const expectEqual = std.testing.expectEqual;
45
const maxInt = std.math.maxInt;
56

67
test "@intCast i32 to u7" {
@@ -28,3 +29,197 @@ test "coerce i8 to i32 and @intCast back" {
2829
var y2: i8 = -5;
2930
try expect(y2 == @as(i8, @intCast(x2)));
3031
}
32+
33+
test "coerce non byte-sized integers accross 32bits boundary" {
34+
{
35+
var v: u21 = 6417;
36+
const a: u32 = v;
37+
const b: u64 = v;
38+
const c: u64 = a;
39+
var w: u64 = 0x1234567812345678;
40+
const d: u21 = @truncate(w);
41+
const e: u60 = d;
42+
try expectEqual(@as(u32, 6417), a);
43+
try expectEqual(@as(u64, 6417), b);
44+
try expectEqual(@as(u64, 6417), c);
45+
try expectEqual(@as(u21, 0x145678), d);
46+
try expectEqual(@as(u60, 0x145678), e);
47+
}
48+
49+
{
50+
var v: u10 = 234;
51+
const a: u32 = v;
52+
const b: u64 = v;
53+
const c: u64 = a;
54+
var w: u64 = 0x1234567812345678;
55+
const d: u10 = @truncate(w);
56+
const e: u60 = d;
57+
try expectEqual(@as(u32, 234), a);
58+
try expectEqual(@as(u64, 234), b);
59+
try expectEqual(@as(u64, 234), c);
60+
try expectEqual(@as(u21, 0x278), d);
61+
try expectEqual(@as(u60, 0x278), e);
62+
}
63+
{
64+
var v: u7 = 11;
65+
const a: u32 = v;
66+
const b: u64 = v;
67+
const c: u64 = a;
68+
var w: u64 = 0x1234567812345678;
69+
const d: u7 = @truncate(w);
70+
const e: u60 = d;
71+
try expectEqual(@as(u32, 11), a);
72+
try expectEqual(@as(u64, 11), b);
73+
try expectEqual(@as(u64, 11), c);
74+
try expectEqual(@as(u21, 0x78), d);
75+
try expectEqual(@as(u60, 0x78), e);
76+
}
77+
78+
{
79+
var v: i21 = -6417;
80+
const a: i32 = v;
81+
const b: i64 = v;
82+
const c: i64 = a;
83+
var w: i64 = -12345;
84+
const d: i21 = @intCast(w);
85+
const e: i60 = d;
86+
try expectEqual(@as(i32, -6417), a);
87+
try expectEqual(@as(i64, -6417), b);
88+
try expectEqual(@as(i64, -6417), c);
89+
try expectEqual(@as(i21, -12345), d);
90+
try expectEqual(@as(i60, -12345), e);
91+
}
92+
93+
{
94+
var v: i10 = -234;
95+
const a: i32 = v;
96+
const b: i64 = v;
97+
const c: i64 = a;
98+
var w: i64 = -456;
99+
const d: i10 = @intCast(w);
100+
const e: i60 = d;
101+
try expectEqual(@as(i32, -234), a);
102+
try expectEqual(@as(i64, -234), b);
103+
try expectEqual(@as(i64, -234), c);
104+
try expectEqual(@as(i10, -456), d);
105+
try expectEqual(@as(i60, -456), e);
106+
}
107+
{
108+
var v: i7 = -11;
109+
const a: i32 = v;
110+
const b: i64 = v;
111+
const c: i64 = a;
112+
var w: i64 = -42;
113+
const d: i7 = @intCast(w);
114+
const e: i60 = d;
115+
try expectEqual(@as(i32, -11), a);
116+
try expectEqual(@as(i64, -11), b);
117+
try expectEqual(@as(i64, -11), c);
118+
try expectEqual(@as(i7, -42), d);
119+
try expectEqual(@as(i60, -42), e);
120+
}
121+
}
122+
123+
const Piece = packed struct {
124+
color: Color,
125+
type: Type,
126+
127+
const Type = enum { KING, QUEEN, BISHOP, KNIGHT, ROOK, PAWN };
128+
const Color = enum { WHITE, BLACK };
129+
130+
fn charToPiece(c: u8) !@This() {
131+
return .{
132+
.type = try charToPieceType(c),
133+
.color = if (std.ascii.isUpper(c)) Color.WHITE else Color.BLACK,
134+
};
135+
}
136+
137+
fn charToPieceType(c: u8) !Type {
138+
return switch (std.ascii.toLower(c)) {
139+
'p' => .PAWN,
140+
'k' => .KING,
141+
'q' => .QUEEN,
142+
'b' => .BISHOP,
143+
'n' => .KNIGHT,
144+
'r' => .ROOK,
145+
else => error.UnexpectedCharError,
146+
};
147+
}
148+
};
149+
150+
test "load non byte-sized optional value" {
151+
// Originally reported at https://github.com/ziglang/zig/issues/14200
152+
// note: this bug is triggered by the == operator, expectEqual will hide it
153+
var opt: ?Piece = try Piece.charToPiece('p');
154+
try expect(opt.?.type == .PAWN);
155+
try expect(opt.?.color == .BLACK);
156+
157+
var p: Piece = undefined;
158+
@as(*u8, @ptrCast(&p)).* = 0b11111011;
159+
try expect(p.type == .PAWN);
160+
try expect(p.color == .BLACK);
161+
}
162+
163+
test "load non byte-sized value in struct" {
164+
if (builtin.cpu.arch.endian() != .Little) return error.SkipZigTest; // packed struct TODO
165+
166+
// note: this bug is triggered by the == operator, expectEqual will hide it
167+
// using ptrCast not to depend on unitialised memory state
168+
169+
var struct0: struct {
170+
p: Piece,
171+
int: u8,
172+
} = undefined;
173+
@as(*u8, @ptrCast(&struct0.p)).* = 0b11111011;
174+
try expect(struct0.p.type == .PAWN);
175+
try expect(struct0.p.color == .BLACK);
176+
177+
var struct1: packed struct {
178+
p0: Piece,
179+
p1: Piece,
180+
pad: u1,
181+
p2: Piece,
182+
} = undefined;
183+
@as(*u8, @ptrCast(&struct1.p0)).* = 0b11111011;
184+
struct1.p1 = try Piece.charToPiece('p');
185+
struct1.p2 = try Piece.charToPiece('p');
186+
try expect(struct1.p0.type == .PAWN);
187+
try expect(struct1.p0.color == .BLACK);
188+
try expect(struct1.p1.type == .PAWN);
189+
try expect(struct1.p1.color == .BLACK);
190+
try expect(struct1.p2.type == .PAWN);
191+
try expect(struct1.p2.color == .BLACK);
192+
}
193+
194+
test "load non byte-sized value in union" {
195+
if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest;
196+
if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest;
197+
if (builtin.zig_backend == .stage2_sparc64) return error.SkipZigTest;
198+
if (builtin.zig_backend == .stage2_spirv64) return error.SkipZigTest;
199+
if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest;
200+
if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest;
201+
202+
// note: this bug is triggered by the == operator, expectEqual will hide it
203+
// using ptrCast not to depend on unitialised memory state
204+
205+
var union0: packed union {
206+
p: Piece,
207+
int: u8,
208+
} = .{ .int = 0 };
209+
union0.int = 0b11111011;
210+
try expect(union0.p.type == .PAWN);
211+
try expect(union0.p.color == .BLACK);
212+
213+
var union1: union {
214+
p: Piece,
215+
int: u8,
216+
} = .{ .p = .{ .color = .WHITE, .type = .KING } };
217+
@as(*u8, @ptrCast(&union1.p)).* = 0b11111011;
218+
try expect(union1.p.type == .PAWN);
219+
try expect(union1.p.color == .BLACK);
220+
221+
var pieces: [3]Piece = undefined;
222+
@as(*u8, @ptrCast(&pieces[1])).* = 0b11111011;
223+
try expect(pieces[1].type == .PAWN);
224+
try expect(pieces[1].color == .BLACK);
225+
}

‎test/behavior/packed-struct.zig

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -991,7 +991,7 @@ test "bitcast back and forth" {
991991

992992
test "field access of packed struct smaller than its abi size inside struct initialized with rls" {
993993
// Originally reported at https://github.com/ziglang/zig/issues/14200
994-
if (builtin.zig_backend == .stage2_llvm and builtin.cpu.arch == .arm) return error.SkipZigTest;
994+
995995
const S = struct {
996996
ps: packed struct { x: i2, y: i2 },
997997

@@ -1036,3 +1036,32 @@ test "modify nested packed struct aligned field" {
10361036
try std.testing.expectEqual(@as(u8, 1), opts.pretty_print.indent);
10371037
try std.testing.expect(!opts.baz);
10381038
}
1039+
1040+
test "assigning packed struct inside another packed struct" {
1041+
// Originally reported at https://github.com/ziglang/zig/issues/9674
1042+
1043+
const S = struct {
1044+
const Inner = packed struct {
1045+
bits: u3,
1046+
more_bits: u6,
1047+
};
1048+
1049+
const Outer = packed struct {
1050+
padding: u5,
1051+
inner: Inner,
1052+
};
1053+
fn t(inner: Inner) void {
1054+
r.inner = inner;
1055+
}
1056+
1057+
var mem: Outer = undefined;
1058+
var r: *volatile Outer = &mem;
1059+
};
1060+
1061+
const val = S.Inner{ .bits = 1, .more_bits = 11 };
1062+
S.mem.padding = 0;
1063+
S.t(val);
1064+
1065+
try expectEqual(val, S.mem.inner);
1066+
try expect(S.mem.padding == 0);
1067+
}

0 commit comments

Comments
 (0)