From a5c910adb610ae530db99f10aa77aaed3e85e830 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 13 Apr 2023 21:44:40 -0700 Subject: change semantics of `@memcpy` and `@memset` Now they use slices or array pointers with any element type instead of requiring byte pointers. This is a breaking enhancement to the language. The safety check for overlapping pointers will be implemented in a future commit. closes #14040 --- src/codegen/llvm.zig | 71 +++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 54 insertions(+), 17 deletions(-) (limited to 'src/codegen') diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index 76dabd3e9b..d80fb62969 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -5776,6 +5776,36 @@ pub const FuncGen = struct { return result; } + fn sliceOrArrayPtr(fg: *FuncGen, ptr: *llvm.Value, ty: Type) *llvm.Value { + switch (ty.ptrSize()) { + .Slice => return fg.builder.buildExtractValue(ptr, 0, ""), + .One => return ptr, + .Many, .C => unreachable, + } + } + + fn sliceOrArrayLenInBytes(fg: *FuncGen, ptr: *llvm.Value, ty: Type) *llvm.Value { + const target = fg.dg.module.getTarget(); + const llvm_usize_ty = fg.context.intType(target.cpu.arch.ptrBitWidth()); + switch (ty.ptrSize()) { + .Slice => { + const len = fg.builder.buildExtractValue(ptr, 1, ""); + const elem_ty = ty.childType(); + const abi_size = elem_ty.abiSize(target); + if (abi_size == 1) return len; + const abi_size_llvm_val = llvm_usize_ty.constInt(abi_size, .False); + return fg.builder.buildMul(len, abi_size_llvm_val, ""); + }, + .One => { + const array_ty = ty.childType(); + const elem_ty = array_ty.childType(); + const abi_size = elem_ty.abiSize(target); + return llvm_usize_ty.constInt(array_ty.arrayLen() * abi_size, .False); + }, + .Many, .C => unreachable, + } + } + fn airSliceField(self: *FuncGen, inst: Air.Inst.Index, index: c_uint) !?*llvm.Value { const ty_op = self.air.instructions.items(.data)[inst].ty_op; const operand = try self.resolveInst(ty_op.operand); @@ -8374,18 +8404,24 @@ pub const FuncGen = struct { } fn airMemset(self: *FuncGen, inst: Air.Inst.Index) !?*llvm.Value { - const pl_op = self.air.instructions.items(.data)[inst].pl_op; - const extra = self.air.extraData(Air.Bin, pl_op.payload).data; - const dest_ptr = try self.resolveInst(pl_op.operand); - const ptr_ty = self.air.typeOf(pl_op.operand); - const value = try self.resolveInst(extra.lhs); - const val_is_undef = if (self.air.value(extra.lhs)) |val| val.isUndefDeep() else false; - const len = try self.resolveInst(extra.rhs); - const u8_llvm_ty = self.context.intType(8); - const fill_char = if (val_is_undef) u8_llvm_ty.constInt(0xaa, .False) else value; + const bin_op = self.air.instructions.items(.data)[inst].bin_op; + const dest_slice = try self.resolveInst(bin_op.lhs); + const ptr_ty = self.air.typeOf(bin_op.lhs); + const value = try self.resolveInst(bin_op.rhs); + const elem_ty = self.air.typeOf(bin_op.rhs); const target = self.dg.module.getTarget(); + const val_is_undef = if (self.air.value(bin_op.rhs)) |val| val.isUndefDeep() else false; + const len = self.sliceOrArrayLenInBytes(dest_slice, ptr_ty); + const dest_ptr = self.sliceOrArrayPtr(dest_slice, ptr_ty); + const u8_llvm_ty = self.context.intType(8); + const fill_byte = if (val_is_undef) u8_llvm_ty.constInt(0xaa, .False) else b: { + if (elem_ty.abiSize(target) != 1) { + return self.dg.todo("implement @memset for non-byte-sized element type", .{}); + } + break :b self.builder.buildBitCast(value, u8_llvm_ty, ""); + }; const dest_ptr_align = ptr_ty.ptrAlignment(target); - _ = self.builder.buildMemSet(dest_ptr, fill_char, len, dest_ptr_align, ptr_ty.isVolatilePtr()); + _ = self.builder.buildMemSet(dest_ptr, fill_byte, len, dest_ptr_align, ptr_ty.isVolatilePtr()); if (val_is_undef and self.dg.module.comp.bin_file.options.valgrind) { self.valgrindMarkUndef(dest_ptr, len); @@ -8394,13 +8430,14 @@ pub const FuncGen = struct { } fn airMemcpy(self: *FuncGen, inst: Air.Inst.Index) !?*llvm.Value { - const pl_op = self.air.instructions.items(.data)[inst].pl_op; - const extra = self.air.extraData(Air.Bin, pl_op.payload).data; - const dest_ptr = try self.resolveInst(pl_op.operand); - const dest_ptr_ty = self.air.typeOf(pl_op.operand); - const src_ptr = try self.resolveInst(extra.lhs); - const src_ptr_ty = self.air.typeOf(extra.lhs); - const len = try self.resolveInst(extra.rhs); + const bin_op = self.air.instructions.items(.data)[inst].bin_op; + const dest_slice = try self.resolveInst(bin_op.lhs); + const dest_ptr_ty = self.air.typeOf(bin_op.lhs); + const src_slice = try self.resolveInst(bin_op.rhs); + const src_ptr_ty = self.air.typeOf(bin_op.rhs); + const src_ptr = self.sliceOrArrayPtr(src_slice, src_ptr_ty); + const len = self.sliceOrArrayLenInBytes(dest_slice, dest_ptr_ty); + const dest_ptr = self.sliceOrArrayPtr(dest_slice, dest_ptr_ty); const is_volatile = src_ptr_ty.isVolatilePtr() or dest_ptr_ty.isVolatilePtr(); const target = self.dg.module.getTarget(); _ = self.builder.buildMemCpy( -- cgit v1.2.3 From edb5e493e6d9a4478b3d9c06aa590694757d8c03 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 21 Apr 2023 18:03:33 -0700 Subject: update `@memcpy` to require equal src and dest lens * Sema: upgrade operands to array pointers if possible when emitting AIR. * Implement safety checks for length mismatch and aliasing. * AIR: make ptrtoint support slice operands. Implement in LLVM backend. * C backend: implement new `@memset` semantics. `@memcpy` is not done yet. --- doc/langref.html.in | 26 +++---- lib/std/builtin.zig | 2 + lib/std/crypto/aegis.zig | 4 +- lib/std/hash/murmur.zig | 11 ++- lib/std/mem/Allocator.zig | 3 +- src/Air.zig | 1 + src/AstGen.zig | 2 +- src/Liveness/Verify.zig | 9 +-- src/Sema.zig | 175 ++++++++++++++++++++++++++++++++++++++++++---- src/codegen/c.zig | 98 +++++++++++++++++++------- src/codegen/llvm.zig | 12 ++-- src/type.zig | 15 ++-- 12 files changed, 280 insertions(+), 78 deletions(-) (limited to 'src/codegen') diff --git a/doc/langref.html.in b/doc/langref.html.in index 7bb60aa348..1fc6a7e98d 100644 --- a/doc/langref.html.in +++ b/doc/langref.html.in @@ -8683,18 +8683,20 @@ test "integer cast panic" { {#header_open|@memcpy#} 
{#syntax#}@memcpy(noalias dest, noalias source) void{#endsyntax#}

This function copies bytes from one region of memory to another.

-

{#syntax#}dest{#endsyntax#} must be a mutable slice, or a mutable pointer to an array. - It may have any alignment, and it may have any element type.

-

{#syntax#}source{#endsyntax#} must be an array, pointer, or a slice - with the same element type as {#syntax#}dest{#endsyntax#}. It may have - any alignment. Only {#syntax#}const{#endsyntax#} access is required. It - is sliced from 0 to the same length as - {#syntax#}dest{#endsyntax#}, triggering the same set of safety checks and - possible compile errors as - {#syntax#}source[0..dest.len]{#endsyntax#}.

-

It is illegal for {#syntax#}dest{#endsyntax#} and - {#syntax#}source[0..dest.len]{#endsyntax#} to overlap. If safety - checks are enabled, there will be a runtime check for such overlapping.

+

{#syntax#}dest{#endsyntax#} must be a mutable slice, a mutable pointer to an array, or + a mutable many-item {#link|pointer|Pointer#}. It may have any + alignment, and it may have any element type.

+

Likewise, {#syntax#}source{#endsyntax#} must be a mutable slice, a + mutable pointer to an array, or a mutable many-item + {#link|pointer|Pointer#}. It may have any alignment, and it may have any + element type.

+

The {#syntax#}source{#endsyntax#} element type must support {#link|Type Coercion#} + into the {#syntax#}dest{#endsyntax#} element type. The element types may have + different ABI size, however, that may incur a performance penalty.

+

Similar to {#link|for#} loops, at least one of {#syntax#}source{#endsyntax#} and + {#syntax#}dest{#endsyntax#} must provide a length, and if two lengths are provided, + they must be equal.

+

Finally, the two memory regions must not overlap.

{#header_close#} {#header_open|@memset#} diff --git a/lib/std/builtin.zig b/lib/std/builtin.zig index cfe22099cb..048f73482e 100644 --- a/lib/std/builtin.zig +++ b/lib/std/builtin.zig @@ -1002,6 +1002,8 @@ pub const panic_messages = struct { pub const index_out_of_bounds = "index out of bounds"; pub const start_index_greater_than_end = "start index is larger than end index"; pub const for_len_mismatch = "for loop over objects with non-equal lengths"; + pub const memcpy_len_mismatch = "@memcpy arguments have non-equal lengths"; + pub const memcpy_alias = "@memcpy arguments alias"; }; pub noinline fn returnError(st: *StackTrace) void { diff --git a/lib/std/crypto/aegis.zig b/lib/std/crypto/aegis.zig index 8cc5a8320e..d7305b444a 100644 --- a/lib/std/crypto/aegis.zig +++ b/lib/std/crypto/aegis.zig @@ -209,7 +209,7 @@ fn Aegis128LGeneric(comptime tag_bits: u9) type { acc |= (computed_tag[j] ^ tag[j]); } if (acc != 0) { - @memset(m.ptr, undefined, m.len); + @memset(m, undefined); return error.AuthenticationFailed; } } @@ -390,7 +390,7 @@ fn Aegis256Generic(comptime tag_bits: u9) type { acc |= (computed_tag[j] ^ tag[j]); } if (acc != 0) { - @memset(m.ptr, undefined, m.len); + @memset(m, undefined); return error.AuthenticationFailed; } } diff --git a/lib/std/hash/murmur.zig b/lib/std/hash/murmur.zig index 15fe1b5d7d..95f7c60efe 100644 --- a/lib/std/hash/murmur.zig +++ b/lib/std/hash/murmur.zig @@ -99,9 +99,8 @@ pub const Murmur2_64 = struct { pub fn hashWithSeed(str: []const u8, seed: u64) u64 { const m: u64 = 0xc6a4a7935bd1e995; - const len = @as(u64, str.len); - var h1: u64 = seed ^ (len *% m); - for (@ptrCast([*]align(1) const u64, str.ptr)[0..@intCast(usize, len >> 3)]) |v| { + var h1: u64 = seed ^ (@as(u64, str.len) *% m); + for (@ptrCast([*]align(1) const u64, str.ptr)[0..str.len / 8]) |v| { var k1: u64 = v; if (native_endian == .Big) k1 = @byteSwap(k1); @@ -111,11 +110,11 @@ pub const Murmur2_64 = struct { h1 ^= k1; h1 *%= m; } - const rest = len & 7; - const offset = len - rest; + const rest = str.len & 7; + const offset = str.len - rest; if (rest > 0) { var k1: u64 = 0; - @memcpy(@ptrCast([*]u8, &k1)[0..@intCast(usize, rest)], @ptrCast([*]const u8, &str[@intCast(usize, offset)])); + @memcpy(@ptrCast([*]u8, &k1)[0..rest], str[offset..]); if (native_endian == .Big) k1 = @byteSwap(k1); h1 ^= k1; diff --git a/lib/std/mem/Allocator.zig b/lib/std/mem/Allocator.zig index ce09cd8baa..b402fab3fa 100644 --- a/lib/std/mem/Allocator.zig +++ b/lib/std/mem/Allocator.zig @@ -282,7 +282,8 @@ pub fn reallocAdvanced( const new_mem = self.rawAlloc(byte_count, log2a(Slice.alignment), return_address) orelse return error.OutOfMemory; - @memcpy(new_mem[0..@min(byte_count, old_byte_slice.len)], old_byte_slice); + const copy_len = @min(byte_count, old_byte_slice.len); + @memcpy(new_mem[0..copy_len], old_byte_slice[0..copy_len]); // TODO https://github.com/ziglang/zig/issues/4298 @memset(old_byte_slice, undefined); self.rawFree(old_byte_slice, log2a(Slice.alignment), return_address); diff --git a/src/Air.zig b/src/Air.zig index c536053f55..4b5d168549 100644 --- a/src/Air.zig +++ b/src/Air.zig @@ -462,6 +462,7 @@ pub const Inst = struct { /// Uses the `ty_op` field. load, /// Converts a pointer to its address. Result type is always `usize`. + /// Pointer type size may be any, including slice. /// Uses the `un_op` field. ptrtoint, /// Given a boolean, returns 0 or 1. diff --git a/src/AstGen.zig b/src/AstGen.zig index 36c03e9ed4..6c9972bc95 100644 --- a/src/AstGen.zig +++ b/src/AstGen.zig @@ -8455,7 +8455,7 @@ fn builtinCall( .memcpy => { _ = try gz.addPlNode(.memcpy, node, Zir.Inst.Bin{ .lhs = try expr(gz, scope, .{ .rl = .none }, params[0]), - .rhs = try expr(gz, scope, .{ .rl = .ref }, params[1]), + .rhs = try expr(gz, scope, .{ .rl = .none }, params[1]), }); return rvalue(gz, ri, .void_value, node); }, diff --git a/src/Liveness/Verify.zig b/src/Liveness/Verify.zig index 6c1e72392d..3a00492f4b 100644 --- a/src/Liveness/Verify.zig +++ b/src/Liveness/Verify.zig @@ -254,6 +254,8 @@ fn verifyBody(self: *Verify, body: []const Air.Inst.Index) Error!void { .set_union_tag, .min, .max, + .memset, + .memcpy, => { const bin_op = data[inst].bin_op; try self.verifyInst(inst, .{ bin_op.lhs, bin_op.rhs, .none }); @@ -306,13 +308,6 @@ fn verifyBody(self: *Verify, body: []const Air.Inst.Index) Error!void { const extra = self.air.extraData(Air.Bin, vector_store_elem.payload).data; try self.verifyInst(inst, .{ vector_store_elem.vector_ptr, extra.lhs, extra.rhs }); }, - .memset, - .memcpy, - => { - const pl_op = data[inst].pl_op; - const extra = self.air.extraData(Air.Bin, pl_op.payload).data; - try self.verifyInst(inst, .{ pl_op.operand, extra.lhs, extra.rhs }); - }, .cmpxchg_strong, .cmpxchg_weak, => { diff --git a/src/Sema.zig b/src/Sema.zig index 46798cde57..1b0ca80f18 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -3386,17 +3386,39 @@ fn zirIndexablePtrLen(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileE const inst_data = sema.code.instructions.items(.data)[inst].un_node; const src = inst_data.src(); const object = try sema.resolveInst(inst_data.operand); - const object_ty = sema.typeOf(object); - - const is_pointer_to = object_ty.isSinglePointer(); - const array_ty = if (is_pointer_to) - object_ty.childType() - else - object_ty; + return indexablePtrLen(sema, block, src, object); +} +fn indexablePtrLen( + sema: *Sema, + block: *Block, + src: LazySrcLoc, + object: Air.Inst.Ref, +) CompileError!Air.Inst.Ref { + const object_ty = sema.typeOf(object); + const is_pointer_to = object_ty.isSinglePointer(); + const array_ty = if (is_pointer_to) object_ty.childType() else object_ty; try checkIndexable(sema, block, src, array_ty); + return sema.fieldVal(block, src, object, "len", src); +} +fn indexablePtrLenOrNone( + sema: *Sema, + block: *Block, + src: LazySrcLoc, + object: Air.Inst.Ref, +) CompileError!Air.Inst.Ref { + const object_ty = sema.typeOf(object); + const array_ty = t: { + const ptr_size = object_ty.ptrSizeOrNull() orelse break :t object_ty; + break :t switch (ptr_size) { + .Many => return .none, + .One => object_ty.childType(), + else => object_ty, + }; + }; + try checkIndexable(sema, block, src, array_ty); return sema.fieldVal(block, src, object, "len", src); } @@ -21773,6 +21795,29 @@ fn analyzeMinMax( return block.addBinOp(air_tag, simd_op.lhs, simd_op.rhs); } +fn upgradeToArrayPtr(sema: *Sema, block: *Block, ptr: Air.Inst.Ref, len: u64) !Air.Inst.Ref { + const mod = sema.mod; + const info = sema.typeOf(ptr).ptrInfo().data; + if (info.size == .One) { + // Already an array pointer. + return ptr; + } + const new_ty = try Type.ptr(sema.arena, mod, .{ + .pointee_type = try Type.array(sema.arena, len, info.sentinel, info.pointee_type, mod), + .sentinel = null, + .@"align" = info.@"align", + .@"addrspace" = info.@"addrspace", + .mutable = info.mutable, + .@"allowzero" = info.@"allowzero", + .@"volatile" = info.@"volatile", + .size = .One, + }); + if (info.size == .Slice) { + return block.addTyOp(.slice_ptr, new_ty, ptr); + } + return block.addBitCast(new_ty, ptr); +} + fn zirMemcpy(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void { const inst_data = sema.code.instructions.items(.data)[inst].pl_node; const extra = sema.code.extraData(Zir.Inst.Bin, inst_data.payload_index).data; @@ -21780,27 +21825,125 @@ fn zirMemcpy(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void const dest_src: LazySrcLoc = .{ .node_offset_builtin_call_arg0 = inst_data.src_node }; const src_src: LazySrcLoc = .{ .node_offset_builtin_call_arg1 = inst_data.src_node }; const dest_ptr = try sema.resolveInst(extra.lhs); - const src_ptr_ptr = try sema.resolveInst(extra.rhs); - const dest_ptr_ty = sema.typeOf(dest_ptr); - try checkSliceOrArrayType(sema, block, dest_src, dest_ptr_ty); + const src_ptr = try sema.resolveInst(extra.rhs); + const dest_len = try indexablePtrLenOrNone(sema, block, dest_src, dest_ptr); + const src_len = try indexablePtrLenOrNone(sema, block, src_src, src_ptr); + + if (dest_len == .none and src_len == .none) { + const msg = msg: { + const msg = try sema.errMsg(block, src, "unknown @memcpy length", .{}); + errdefer msg.destroy(sema.gpa); + try sema.errNote(block, dest_src, msg, "destination type {} provides no length", .{ + sema.typeOf(dest_ptr).fmt(sema.mod), + }); + try sema.errNote(block, src_src, msg, "source type {} provides no length", .{ + sema.typeOf(src_ptr).fmt(sema.mod), + }); + break :msg msg; + }; + return sema.failWithOwnedErrorMsg(msg); + } - const dest_len = try sema.fieldVal(block, dest_src, dest_ptr, "len", dest_src); - const src_ptr = try sema.analyzeSlice(block, src_src, src_ptr_ptr, .zero_usize, dest_len, .none, .unneeded, src_src, src_src, src_src); + var len_val: ?Value = null; + + if (dest_len != .none and src_len != .none) check: { + // If we can check at compile-time, no need for runtime safety. + if (try sema.resolveDefinedValue(block, dest_src, dest_len)) |dest_len_val| { + len_val = dest_len_val; + if (try sema.resolveDefinedValue(block, src_src, src_len)) |src_len_val| { + if (!(try sema.valuesEqual(dest_len_val, src_len_val, Type.usize))) { + const msg = msg: { + const msg = try sema.errMsg(block, src, "non-matching @memcpy lengths", .{}); + errdefer msg.destroy(sema.gpa); + try sema.errNote(block, dest_src, msg, "length {} here", .{ + dest_len_val.fmtValue(Type.usize, sema.mod), + }); + try sema.errNote(block, src_src, msg, "length {} here", .{ + src_len_val.fmtValue(Type.usize, sema.mod), + }); + break :msg msg; + }; + return sema.failWithOwnedErrorMsg(msg); + } + break :check; + } + } else if (try sema.resolveDefinedValue(block, src_src, src_len)) |src_len_val| { + len_val = src_len_val; + } + + if (block.wantSafety()) { + const ok = try block.addBinOp(.cmp_eq, dest_len, src_len); + try sema.addSafetyCheck(block, ok, .memcpy_len_mismatch); + } + } const runtime_src = if (try sema.resolveDefinedValue(block, dest_src, dest_ptr)) |dest_ptr_val| rs: { if (!dest_ptr_val.isComptimeMutablePtr()) break :rs dest_src; if (try sema.resolveDefinedValue(block, src_src, src_ptr)) |src_ptr_val| { - if (!src_ptr_val.isComptimeMutablePtr()) break :rs src_src; + _ = src_ptr_val; return sema.fail(block, src, "TODO: @memcpy at comptime", .{}); } else break :rs src_src; } else dest_src; try sema.requireRuntimeBlock(block, src, runtime_src); + + const dest_ty = sema.typeOf(dest_ptr); + const src_ty = sema.typeOf(src_ptr); + + // If in-memory coercion is not allowed, explode this memcpy call into a + // for loop that copies element-wise. + // Likewise if this is an iterable rather than a pointer, do the same + // lowering. The AIR instruction requires pointers with element types of + // equal ABI size. + + if (dest_ty.zigTypeTag() != .Pointer or src_ty.zigTypeTag() != .Pointer) { + return sema.fail(block, src, "TODO: lower @memcpy to a for loop because the source or destination iterable is a tuple", .{}); + } + + const dest_elem_ty = dest_ty.elemType2(); + const src_elem_ty = src_ty.elemType2(); + const target = sema.mod.getTarget(); + if (.ok != try sema.coerceInMemoryAllowed(block, dest_elem_ty, src_elem_ty, true, target, dest_src, src_src)) { + return sema.fail(block, src, "TODO: lower @memcpy to a for loop because the element types have different ABI sizes", .{}); + } + + // If the length is comptime-known, then upgrade src and destination types + // into pointer-to-array. At this point we know they are both pointers + // already. + var new_dest_ptr = dest_ptr; + var new_src_ptr = src_ptr; + if (len_val) |val| { + const len = val.toUnsignedInt(target); + new_dest_ptr = try upgradeToArrayPtr(sema, block, dest_ptr, len); + new_src_ptr = try upgradeToArrayPtr(sema, block, src_ptr, len); + } + + // Aliasing safety check. + if (block.wantSafety()) { + const dest_int = try block.addUnOp(.ptrtoint, new_dest_ptr); + const src_int = try block.addUnOp(.ptrtoint, new_src_ptr); + const len = if (len_val) |v| + try sema.addConstant(Type.usize, v) + else if (dest_len != .none) + dest_len + else + src_len; + + // ok1: dest >= src + len + // ok2: src >= dest + len + const src_plus_len = try block.addBinOp(.add, src_int, len); + const dest_plus_len = try block.addBinOp(.add, dest_int, len); + const ok1 = try block.addBinOp(.cmp_gte, dest_int, src_plus_len); + const ok2 = try block.addBinOp(.cmp_gte, src_int, dest_plus_len); + const ok = try block.addBinOp(.bit_or, ok1, ok2); + try sema.addSafetyCheck(block, ok, .memcpy_alias); + } + _ = try block.addInst(.{ .tag = .memcpy, .data = .{ .bin_op = .{ - .lhs = dest_ptr, - .rhs = src_ptr, + .lhs = new_dest_ptr, + .rhs = new_src_ptr, } }, }); } @@ -22949,6 +23092,8 @@ pub const PanicId = enum { index_out_of_bounds, start_index_greater_than_end, for_len_mismatch, + memcpy_len_mismatch, + memcpy_alias, }; fn addSafetyCheck( diff --git a/src/codegen/c.zig b/src/codegen/c.zig index 903d2449fd..9dac87f11b 100644 --- a/src/codegen/c.zig +++ b/src/codegen/c.zig @@ -6177,18 +6177,43 @@ fn airAtomicStore(f: *Function, inst: Air.Inst.Index, order: [*:0]const u8) !CVa } fn airMemset(f: *Function, inst: Air.Inst.Index) !CValue { - const pl_op = f.air.instructions.items(.data)[inst].pl_op; - const extra = f.air.extraData(Air.Bin, pl_op.payload).data; - const dest_ty = f.air.typeOf(pl_op.operand); - const dest_ptr = try f.resolveInst(pl_op.operand); - const value = try f.resolveInst(extra.lhs); - const len = try f.resolveInst(extra.rhs); - + const bin_op = f.air.instructions.items(.data)[inst].bin_op; + const dest_ty = f.air.typeOf(bin_op.lhs); + const dest_slice = try f.resolveInst(bin_op.lhs); + const value = try f.resolveInst(bin_op.rhs); + const elem_ty = f.air.typeOf(bin_op.rhs); + const target = f.object.dg.module.getTarget(); + const elem_abi_size = elem_ty.abiSize(target); + const val_is_undef = if (f.air.value(bin_op.rhs)) |val| val.isUndefDeep() else false; const writer = f.object.writer(); - if (dest_ty.isVolatilePtr()) { - var u8_ptr_pl = dest_ty.ptrInfo(); - u8_ptr_pl.data.pointee_type = Type.u8; - const u8_ptr_ty = Type.initPayload(&u8_ptr_pl.base); + + if (val_is_undef) { + try writer.writeAll("memset("); + switch (dest_ty.ptrSize()) { + .Slice => { + try f.writeCValueMember(writer, dest_slice, .{ .identifier = "ptr" }); + try writer.writeAll(", 0xaa, "); + try f.writeCValueMember(writer, dest_slice, .{ .identifier = "len" }); + if (elem_abi_size > 1) { + try writer.print(" * {d});\n", .{elem_abi_size}); + } else { + try writer.writeAll(");\n"); + } + }, + .One => { + const array_ty = dest_ty.childType(); + const len = array_ty.arrayLen() * elem_abi_size; + + try f.writeCValue(writer, dest_slice, .FunctionArgument); + try writer.print(", 0xaa, {d});\n", .{len}); + }, + .Many, .C => unreachable, + } + try reap(f, inst, &.{ bin_op.lhs, bin_op.rhs }); + return .none; + } + + if (elem_abi_size > 1 or dest_ty.isVolatilePtr()) { const index = try f.allocLocal(inst, Type.usize); try writer.writeAll("for ("); @@ -6198,36 +6223,61 @@ fn airMemset(f: *Function, inst: Air.Inst.Index) !CValue { try writer.writeAll("; "); try f.writeCValue(writer, index, .Other); try writer.writeAll(" != "); - try f.writeCValue(writer, len, .Other); + switch (dest_ty.ptrSize()) { + .Slice => { + try f.writeCValueMember(writer, dest_slice, .{ .identifier = "len" }); + }, + .One => { + const array_ty = dest_ty.childType(); + const len = array_ty.arrayLen() * elem_abi_size; + try writer.print("{d}", .{len}); + }, + .Many, .C => unreachable, + } try writer.writeAll("; "); try f.writeCValue(writer, index, .Other); try writer.writeAll(" += "); try f.object.dg.renderValue(writer, Type.usize, Value.one, .Other); - try writer.writeAll(") (("); - try f.renderType(writer, u8_ptr_ty); - try writer.writeByte(')'); - try f.writeCValue(writer, dest_ptr, .FunctionArgument); + try writer.writeAll(") ("); + switch (dest_ty.ptrSize()) { + .Slice => try f.writeCValueMember(writer, dest_slice, .{ .identifier = "ptr" }), + .One => try f.writeCValue(writer, dest_slice, .FunctionArgument), + .Many, .C => unreachable, + } try writer.writeAll(")["); try f.writeCValue(writer, index, .Other); try writer.writeAll("] = "); try f.writeCValue(writer, value, .FunctionArgument); try writer.writeAll(";\n"); - try reap(f, inst, &.{ pl_op.operand, extra.lhs, extra.rhs }); + try reap(f, inst, &.{ bin_op.lhs, bin_op.rhs }); try freeLocal(f, inst, index.new_local, 0); return .none; } - try reap(f, inst, &.{ pl_op.operand, extra.lhs, extra.rhs }); try writer.writeAll("memset("); - try f.writeCValue(writer, dest_ptr, .FunctionArgument); - try writer.writeAll(", "); - try f.writeCValue(writer, value, .FunctionArgument); - try writer.writeAll(", "); - try f.writeCValue(writer, len, .FunctionArgument); - try writer.writeAll(");\n"); + switch (dest_ty.ptrSize()) { + .Slice => { + try f.writeCValueMember(writer, dest_slice, .{ .identifier = "ptr" }); + try writer.writeAll(", "); + try f.writeCValue(writer, value, .FunctionArgument); + try writer.writeAll(", "); + try f.writeCValueMember(writer, dest_slice, .{ .identifier = "len" }); + try writer.writeAll(");\n"); + }, + .One => { + const array_ty = dest_ty.childType(); + const len = array_ty.arrayLen() * elem_abi_size; + try f.writeCValue(writer, dest_slice, .FunctionArgument); + try writer.writeAll(", "); + try f.writeCValue(writer, value, .FunctionArgument); + try writer.print(", {d});\n", .{len}); + }, + .Many, .C => unreachable, + } + try reap(f, inst, &.{ bin_op.lhs, bin_op.rhs }); return .none; } diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index d80fb62969..50db9c76f8 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -5777,10 +5777,10 @@ pub const FuncGen = struct { } fn sliceOrArrayPtr(fg: *FuncGen, ptr: *llvm.Value, ty: Type) *llvm.Value { - switch (ty.ptrSize()) { - .Slice => return fg.builder.buildExtractValue(ptr, 0, ""), - .One => return ptr, - .Many, .C => unreachable, + if (ty.isSlice()) { + return fg.builder.buildExtractValue(ptr, 0, ""); + } else { + return ptr; } } @@ -7917,8 +7917,10 @@ pub const FuncGen = struct { fn airPtrToInt(self: *FuncGen, inst: Air.Inst.Index) !?*llvm.Value { const un_op = self.air.instructions.items(.data)[inst].un_op; const operand = try self.resolveInst(un_op); + const ptr_ty = self.air.typeOf(un_op); + const operand_ptr = self.sliceOrArrayPtr(operand, ptr_ty); const dest_llvm_ty = try self.dg.lowerType(self.air.typeOfIndex(inst)); - return self.builder.buildPtrToInt(operand, dest_llvm_ty, ""); + return self.builder.buildPtrToInt(operand_ptr, dest_llvm_ty, ""); } fn airBitCast(self: *FuncGen, inst: Air.Inst.Index) !?*llvm.Value { diff --git a/src/type.zig b/src/type.zig index 00bbcac42f..c7b2844970 100644 --- a/src/type.zig +++ b/src/type.zig @@ -3843,9 +3843,14 @@ pub const Type = extern union { }; } - /// Asserts the `Type` is a pointer. - pub fn ptrSize(self: Type) std.builtin.Type.Pointer.Size { - return switch (self.tag()) { + /// Asserts `ty` is a pointer. + pub fn ptrSize(ty: Type) std.builtin.Type.Pointer.Size { + return ptrSizeOrNull(ty).?; + } + + /// Returns `null` if `ty` is not a pointer. + pub fn ptrSizeOrNull(ty: Type) ?std.builtin.Type.Pointer.Size { + return switch (ty.tag()) { .const_slice, .mut_slice, .const_slice_u8, @@ -3870,9 +3875,9 @@ pub const Type = extern union { .inferred_alloc_mut, => .One, - .pointer => self.castTag(.pointer).?.data.size, + .pointer => ty.castTag(.pointer).?.data.size, - else => unreachable, + else => null, }; } -- cgit v1.2.3 From 92186b8c137d14917dc058f228be351f658c1e7e Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 21 Apr 2023 20:11:43 -0700 Subject: C backend: implement new memcpy and inttoptr semantics --- src/codegen/c.zig | 60 ++++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 44 insertions(+), 16 deletions(-) (limited to 'src/codegen') diff --git a/src/codegen/c.zig b/src/codegen/c.zig index 9dac87f11b..332db9dc65 100644 --- a/src/codegen/c.zig +++ b/src/codegen/c.zig @@ -5774,6 +5774,7 @@ fn airPtrToInt(f: *Function, inst: Air.Inst.Index) !CValue { const un_op = f.air.instructions.items(.data)[inst].un_op; const operand = try f.resolveInst(un_op); + const operand_ty = f.air.typeOf(un_op); try reap(f, inst, &.{un_op}); const inst_ty = f.air.typeOfIndex(inst); const writer = f.object.writer(); @@ -5783,7 +5784,11 @@ fn airPtrToInt(f: *Function, inst: Air.Inst.Index) !CValue { try writer.writeAll(" = ("); try f.renderType(writer, inst_ty); try writer.writeByte(')'); - try f.writeCValue(writer, operand, .Other); + if (operand_ty.isSlice()) { + try f.writeCValueMember(writer, operand, .{ .identifier = "len" }); + } else { + try f.writeCValue(writer, operand, .Other); + } try writer.writeAll(";\n"); return local; } @@ -6176,6 +6181,14 @@ fn airAtomicStore(f: *Function, inst: Air.Inst.Index, order: [*:0]const u8) !CVa return .none; } +fn writeSliceOrPtr(f: *Function, writer: anytype, ptr: CValue, ptr_ty: Type) !void { + if (ptr_ty.isSlice()) { + try f.writeCValueMember(writer, ptr, .{ .identifier = "ptr" }); + } else { + try f.writeCValue(writer, ptr, .FunctionArgument); + } +} + fn airMemset(f: *Function, inst: Air.Inst.Index) !CValue { const bin_op = f.air.instructions.items(.data)[inst].bin_op; const dest_ty = f.air.typeOf(bin_op.lhs); @@ -6239,11 +6252,7 @@ fn airMemset(f: *Function, inst: Air.Inst.Index) !CValue { try writer.writeAll(" += "); try f.object.dg.renderValue(writer, Type.usize, Value.one, .Other); try writer.writeAll(") ("); - switch (dest_ty.ptrSize()) { - .Slice => try f.writeCValueMember(writer, dest_slice, .{ .identifier = "ptr" }), - .One => try f.writeCValue(writer, dest_slice, .FunctionArgument), - .Many, .C => unreachable, - } + try writeSliceOrPtr(f, writer, dest_slice, dest_ty); try writer.writeAll(")["); try f.writeCValue(writer, index, .Other); try writer.writeAll("] = "); @@ -6282,22 +6291,41 @@ fn airMemset(f: *Function, inst: Air.Inst.Index) !CValue { } fn airMemcpy(f: *Function, inst: Air.Inst.Index) !CValue { - const pl_op = f.air.instructions.items(.data)[inst].pl_op; - const extra = f.air.extraData(Air.Bin, pl_op.payload).data; - const dest_ptr = try f.resolveInst(pl_op.operand); - const src_ptr = try f.resolveInst(extra.lhs); - const len = try f.resolveInst(extra.rhs); - try reap(f, inst, &.{ pl_op.operand, extra.lhs, extra.rhs }); + const bin_op = f.air.instructions.items(.data)[inst].bin_op; + const dest_ptr = try f.resolveInst(bin_op.lhs); + const src_ptr = try f.resolveInst(bin_op.rhs); + const dest_ty = f.air.typeOf(bin_op.lhs); + const src_ty = f.air.typeOf(bin_op.rhs); + const target = f.object.dg.module.getTarget(); const writer = f.object.writer(); try writer.writeAll("memcpy("); - try f.writeCValue(writer, dest_ptr, .FunctionArgument); + try writeSliceOrPtr(f, writer, dest_ptr, dest_ty); try writer.writeAll(", "); - try f.writeCValue(writer, src_ptr, .FunctionArgument); + try writeSliceOrPtr(f, writer, src_ptr, src_ty); try writer.writeAll(", "); - try f.writeCValue(writer, len, .FunctionArgument); - try writer.writeAll(");\n"); + switch (dest_ty.ptrSize()) { + .Slice => { + const elem_ty = dest_ty.childType(); + const elem_abi_size = elem_ty.abiSize(target); + try f.writeCValueMember(writer, dest_ptr, .{ .identifier = "len" }); + if (elem_abi_size > 1) { + try writer.print(" * {d});\n", .{elem_abi_size}); + } else { + try writer.writeAll(");\n"); + } + }, + .One => { + const array_ty = dest_ty.childType(); + const elem_ty = array_ty.childType(); + const elem_abi_size = elem_ty.abiSize(target); + const len = array_ty.arrayLen() * elem_abi_size; + try writer.print("{d});\n", .{len}); + }, + .Many, .C => unreachable, + } + try reap(f, inst, &.{ bin_op.lhs, bin_op.rhs }); return .none; } -- cgit v1.2.3 From 057c950093085e392fcdd6d6c8e7fb4356dd9959 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 21 Apr 2023 23:05:36 -0700 Subject: LLVM backend: support non-byte-sized memset Also introduce memset_safe AIR tag and support it in C backend and LLVM backend. --- src/Air.zig | 9 +++++ src/Liveness.zig | 2 + src/Liveness/Verify.zig | 1 + src/Sema.zig | 2 +- src/arch/aarch64/CodeGen.zig | 10 ++++- src/arch/arm/CodeGen.zig | 10 ++++- src/arch/riscv64/CodeGen.zig | 10 ++++- src/arch/sparc64/CodeGen.zig | 10 ++++- src/arch/wasm/CodeGen.zig | 3 +- src/arch/x86_64/CodeGen.zig | 11 ++++- src/codegen/c.zig | 13 ++++-- src/codegen/llvm.zig | 95 +++++++++++++++++++++++++++++++++++++------- src/print_air.zig | 1 + test/behavior/basic.zig | 44 ++++++++++++++++++++ 14 files changed, 191 insertions(+), 30 deletions(-) (limited to 'src/codegen') diff --git a/src/Air.zig b/src/Air.zig index 4b5d168549..290123cee6 100644 --- a/src/Air.zig +++ b/src/Air.zig @@ -638,7 +638,14 @@ pub const Inst = struct { /// The element type may be any type, and the slice may have any alignment. /// Result type is always void. /// Uses the `bin_op` field. LHS is the dest slice. RHS is the element value. + /// The element value may be undefined, in which case the destination + /// memory region has undefined bytes after this function executes. In + /// such case ignoring this instruction is legal lowering. memset, + /// Same as `memset`, except if the element value is undefined, the memory region + /// should be filled with 0xaa bytes, and any other safety metadata such as Valgrind + /// integrations should be notified of this memory region being undefined. + memset_safe, /// Given dest pointer and source pointer, copy elements from source to dest. /// Dest pointer is either a slice or a pointer to array. /// The dest element type may be any type. @@ -1236,6 +1243,7 @@ pub fn typeOfIndex(air: Air, inst: Air.Inst.Index) Type { .atomic_store_release, .atomic_store_seq_cst, .memset, + .memset_safe, .memcpy, .set_union_tag, .prefetch, @@ -1415,6 +1423,7 @@ pub fn mustLower(air: Air, inst: Air.Inst.Index) bool { .errunion_payload_ptr_set, .set_union_tag, .memset, + .memset_safe, .memcpy, .cmpxchg_weak, .cmpxchg_strong, diff --git a/src/Liveness.zig b/src/Liveness.zig index 970cd22a00..32ba6927a4 100644 --- a/src/Liveness.zig +++ b/src/Liveness.zig @@ -305,6 +305,7 @@ pub fn categorizeOperand( .atomic_store_seq_cst, .set_union_tag, .memset, + .memset_safe, .memcpy, => { const o = air_datas[inst].bin_op; @@ -980,6 +981,7 @@ fn analyzeInst( .min, .max, .memset, + .memset_safe, .memcpy, => { const o = inst_datas[inst].bin_op; diff --git a/src/Liveness/Verify.zig b/src/Liveness/Verify.zig index 3a00492f4b..41910485ef 100644 --- a/src/Liveness/Verify.zig +++ b/src/Liveness/Verify.zig @@ -255,6 +255,7 @@ fn verifyBody(self: *Verify, body: []const Air.Inst.Index) Error!void { .min, .max, .memset, + .memset_safe, .memcpy, => { const bin_op = data[inst].bin_op; diff --git a/src/Sema.zig b/src/Sema.zig index 1b0ca80f18..5e16b9f3e5 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -21972,7 +21972,7 @@ fn zirMemset(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void try sema.requireRuntimeBlock(block, src, runtime_src); _ = try block.addInst(.{ - .tag = .memset, + .tag = if (block.wantSafety()) .memset_safe else .memset, .data = .{ .bin_op = .{ .lhs = dest_ptr, .rhs = elem, diff --git a/src/arch/aarch64/CodeGen.zig b/src/arch/aarch64/CodeGen.zig index a2db3459dc..817efc32c6 100644 --- a/src/arch/aarch64/CodeGen.zig +++ b/src/arch/aarch64/CodeGen.zig @@ -775,7 +775,8 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void { .atomic_rmw => try self.airAtomicRmw(inst), .atomic_load => try self.airAtomicLoad(inst), .memcpy => try self.airMemcpy(inst), - .memset => try self.airMemset(inst), + .memset => try self.airMemset(inst, false), + .memset_safe => try self.airMemset(inst, true), .set_union_tag => try self.airSetUnionTag(inst), .get_union_tag => try self.airGetUnionTag(inst), .clz => try self.airClz(inst), @@ -5975,8 +5976,13 @@ fn airAtomicStore(self: *Self, inst: Air.Inst.Index, order: std.builtin.AtomicOr return self.fail("TODO implement airAtomicStore for {}", .{self.target.cpu.arch}); } -fn airMemset(self: *Self, inst: Air.Inst.Index) !void { +fn airMemset(self: *Self, inst: Air.Inst.Index, safety: bool) !void { _ = inst; + if (safety) { + // TODO if the value is undef, write 0xaa bytes to dest + } else { + // TODO if the value is undef, don't lower this instruction + } return self.fail("TODO implement airMemset for {}", .{self.target.cpu.arch}); } diff --git a/src/arch/arm/CodeGen.zig b/src/arch/arm/CodeGen.zig index 156ad380b8..bdbe645878 100644 --- a/src/arch/arm/CodeGen.zig +++ b/src/arch/arm/CodeGen.zig @@ -759,7 +759,8 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void { .atomic_rmw => try self.airAtomicRmw(inst), .atomic_load => try self.airAtomicLoad(inst), .memcpy => try self.airMemcpy(inst), - .memset => try self.airMemset(inst), + .memset => try self.airMemset(inst, false), + .memset_safe => try self.airMemset(inst, true), .set_union_tag => try self.airSetUnionTag(inst), .get_union_tag => try self.airGetUnionTag(inst), .clz => try self.airClz(inst), @@ -5921,7 +5922,12 @@ fn airAtomicStore(self: *Self, inst: Air.Inst.Index, order: std.builtin.AtomicOr return self.fail("TODO implement airAtomicStore for {}", .{self.target.cpu.arch}); } -fn airMemset(self: *Self, inst: Air.Inst.Index) !void { +fn airMemset(self: *Self, inst: Air.Inst.Index, safety: bool) !void { + if (safety) { + // TODO if the value is undef, write 0xaa bytes to dest + } else { + // TODO if the value is undef, don't lower this instruction + } _ = inst; return self.fail("TODO implement airMemset for {}", .{self.target.cpu.arch}); } diff --git a/src/arch/riscv64/CodeGen.zig b/src/arch/riscv64/CodeGen.zig index e7dce48dbf..53063fa1dc 100644 --- a/src/arch/riscv64/CodeGen.zig +++ b/src/arch/riscv64/CodeGen.zig @@ -589,7 +589,8 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void { .atomic_rmw => try self.airAtomicRmw(inst), .atomic_load => try self.airAtomicLoad(inst), .memcpy => try self.airMemcpy(inst), - .memset => try self.airMemset(inst), + .memset => try self.airMemset(inst, false), + .memset_safe => try self.airMemset(inst, true), .set_union_tag => try self.airSetUnionTag(inst), .get_union_tag => try self.airGetUnionTag(inst), .clz => try self.airClz(inst), @@ -2421,8 +2422,13 @@ fn airAtomicStore(self: *Self, inst: Air.Inst.Index, order: std.builtin.AtomicOr return self.fail("TODO implement airAtomicStore for {}", .{self.target.cpu.arch}); } -fn airMemset(self: *Self, inst: Air.Inst.Index) !void { +fn airMemset(self: *Self, inst: Air.Inst.Index, safety: bool) !void { _ = inst; + if (safety) { + // TODO if the value is undef, write 0xaa bytes to dest + } else { + // TODO if the value is undef, don't lower this instruction + } return self.fail("TODO implement airMemset for {}", .{self.target.cpu.arch}); } diff --git a/src/arch/sparc64/CodeGen.zig b/src/arch/sparc64/CodeGen.zig index beb2ce2fd2..53e07b2103 100644 --- a/src/arch/sparc64/CodeGen.zig +++ b/src/arch/sparc64/CodeGen.zig @@ -605,7 +605,8 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void { .atomic_rmw => try self.airAtomicRmw(inst), .atomic_load => try self.airAtomicLoad(inst), .memcpy => @panic("TODO try self.airMemcpy(inst)"), - .memset => try self.airMemset(inst), + .memset => try self.airMemset(inst, false), + .memset_safe => try self.airMemset(inst, true), .set_union_tag => try self.airSetUnionTag(inst), .get_union_tag => try self.airGetUnionTag(inst), .clz => try self.airClz(inst), @@ -1764,7 +1765,12 @@ fn airLoop(self: *Self, inst: Air.Inst.Index) !void { return self.finishAirBookkeeping(); } -fn airMemset(self: *Self, inst: Air.Inst.Index) !void { +fn airMemset(self: *Self, inst: Air.Inst.Index, safety: bool) !void { + if (safety) { + // TODO if the value is undef, write 0xaa bytes to dest + } else { + // TODO if the value is undef, don't lower this instruction + } const pl_op = self.air.instructions.items(.data)[inst].pl_op; const extra = self.air.extraData(Air.Bin, pl_op.payload); diff --git a/src/arch/wasm/CodeGen.zig b/src/arch/wasm/CodeGen.zig index 740e95d80d..e3f07d0606 100644 --- a/src/arch/wasm/CodeGen.zig +++ b/src/arch/wasm/CodeGen.zig @@ -1883,7 +1883,8 @@ fn genInst(func: *CodeGen, inst: Air.Inst.Index) InnerError!void { .load => func.airLoad(inst), .loop => func.airLoop(inst), - .memset => func.airMemset(inst), + // TODO: elide memset when writing undef without safety + .memset, .memset_safe => func.airMemset(inst), .not => func.airNot(inst), .optional_payload => func.airOptionalPayload(inst), .optional_payload_ptr => func.airOptionalPayloadPtr(inst), diff --git a/src/arch/x86_64/CodeGen.zig b/src/arch/x86_64/CodeGen.zig index 28085f094f..5738a4fa87 100644 --- a/src/arch/x86_64/CodeGen.zig +++ b/src/arch/x86_64/CodeGen.zig @@ -1046,7 +1046,8 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void { .atomic_rmw => try self.airAtomicRmw(inst), .atomic_load => try self.airAtomicLoad(inst), .memcpy => try self.airMemcpy(inst), - .memset => try self.airMemset(inst), + .memset => try self.airMemset(inst, false), + .memset_safe => try self.airMemset(inst, true), .set_union_tag => try self.airSetUnionTag(inst), .get_union_tag => try self.airGetUnionTag(inst), .clz => try self.airClz(inst), @@ -8149,7 +8150,13 @@ fn airAtomicStore(self: *Self, inst: Air.Inst.Index, order: std.builtin.AtomicOr return self.finishAir(inst, result, .{ bin_op.lhs, bin_op.rhs, .none }); } -fn airMemset(self: *Self, inst: Air.Inst.Index) !void { +fn airMemset(self: *Self, inst: Air.Inst.Index, safety: bool) !void { + if (safety) { + // TODO if the value is undef, write 0xaa bytes to dest + } else { + // TODO if the value is undef, don't lower this instruction + } + const bin_op = self.air.instructions.items(.data)[inst].bin_op; const dst_ptr = try self.resolveInst(bin_op.lhs); diff --git a/src/codegen/c.zig b/src/codegen/c.zig index 332db9dc65..2823cbef05 100644 --- a/src/codegen/c.zig +++ b/src/codegen/c.zig @@ -2925,7 +2925,8 @@ fn genBodyInner(f: *Function, body: []const Air.Inst.Index) error{ AnalysisFail, .cmpxchg_strong => try airCmpxchg(f, inst, "strong"), .atomic_rmw => try airAtomicRmw(f, inst), .atomic_load => try airAtomicLoad(f, inst), - .memset => try airMemset(f, inst), + .memset => try airMemset(f, inst, false), + .memset_safe => try airMemset(f, inst, true), .memcpy => try airMemcpy(f, inst), .set_union_tag => try airSetUnionTag(f, inst), .get_union_tag => try airGetUnionTag(f, inst), @@ -6189,7 +6190,7 @@ fn writeSliceOrPtr(f: *Function, writer: anytype, ptr: CValue, ptr_ty: Type) !vo } } -fn airMemset(f: *Function, inst: Air.Inst.Index) !CValue { +fn airMemset(f: *Function, inst: Air.Inst.Index, safety: bool) !CValue { const bin_op = f.air.instructions.items(.data)[inst].bin_op; const dest_ty = f.air.typeOf(bin_op.lhs); const dest_slice = try f.resolveInst(bin_op.lhs); @@ -6201,6 +6202,11 @@ fn airMemset(f: *Function, inst: Air.Inst.Index) !CValue { const writer = f.object.writer(); if (val_is_undef) { + if (!safety) { + try reap(f, inst, &.{ bin_op.lhs, bin_op.rhs }); + return .none; + } + try writer.writeAll("memset("); switch (dest_ty.ptrSize()) { .Slice => { @@ -6242,8 +6248,7 @@ fn airMemset(f: *Function, inst: Air.Inst.Index) !CValue { }, .One => { const array_ty = dest_ty.childType(); - const len = array_ty.arrayLen() * elem_abi_size; - try writer.print("{d}", .{len}); + try writer.print("{d}", .{array_ty.arrayLen()}); }, .Many, .C => unreachable, } diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index 50db9c76f8..e8ed5745d6 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -4672,7 +4672,8 @@ pub const FuncGen = struct { .fence => try self.airFence(inst), .atomic_rmw => try self.airAtomicRmw(inst), .atomic_load => try self.airAtomicLoad(inst), - .memset => try self.airMemset(inst), + .memset => try self.airMemset(inst, false), + .memset_safe => try self.airMemset(inst, true), .memcpy => try self.airMemcpy(inst), .set_union_tag => try self.airSetUnionTag(inst), .get_union_tag => try self.airGetUnionTag(inst), @@ -8405,29 +8406,95 @@ pub const FuncGen = struct { return null; } - fn airMemset(self: *FuncGen, inst: Air.Inst.Index) !?*llvm.Value { + fn airMemset(self: *FuncGen, inst: Air.Inst.Index, safety: bool) !?*llvm.Value { const bin_op = self.air.instructions.items(.data)[inst].bin_op; const dest_slice = try self.resolveInst(bin_op.lhs); const ptr_ty = self.air.typeOf(bin_op.lhs); - const value = try self.resolveInst(bin_op.rhs); const elem_ty = self.air.typeOf(bin_op.rhs); const target = self.dg.module.getTarget(); const val_is_undef = if (self.air.value(bin_op.rhs)) |val| val.isUndefDeep() else false; - const len = self.sliceOrArrayLenInBytes(dest_slice, ptr_ty); - const dest_ptr = self.sliceOrArrayPtr(dest_slice, ptr_ty); + const dest_ptr_align = ptr_ty.ptrAlignment(target); const u8_llvm_ty = self.context.intType(8); - const fill_byte = if (val_is_undef) u8_llvm_ty.constInt(0xaa, .False) else b: { - if (elem_ty.abiSize(target) != 1) { - return self.dg.todo("implement @memset for non-byte-sized element type", .{}); + const dest_ptr = self.sliceOrArrayPtr(dest_slice, ptr_ty); + + if (val_is_undef) { + // Even if safety is disabled, we still emit a memset to undefined since it conveys + // extra information to LLVM. However, safety makes the difference between using + // 0xaa or actual undefined for the fill byte. + const fill_byte = if (safety) + u8_llvm_ty.constInt(0xaa, .False) + else + u8_llvm_ty.getUndef(); + const len = self.sliceOrArrayLenInBytes(dest_slice, ptr_ty); + _ = self.builder.buildMemSet(dest_ptr, fill_byte, len, dest_ptr_align, ptr_ty.isVolatilePtr()); + + if (safety and self.dg.module.comp.bin_file.options.valgrind) { + self.valgrindMarkUndef(dest_ptr, len); } - break :b self.builder.buildBitCast(value, u8_llvm_ty, ""); - }; - const dest_ptr_align = ptr_ty.ptrAlignment(target); - _ = self.builder.buildMemSet(dest_ptr, fill_byte, len, dest_ptr_align, ptr_ty.isVolatilePtr()); + return null; + } + + const value = try self.resolveInst(bin_op.rhs); + const elem_abi_size = elem_ty.abiSize(target); - if (val_is_undef and self.dg.module.comp.bin_file.options.valgrind) { - self.valgrindMarkUndef(dest_ptr, len); + if (elem_abi_size == 1) { + // In this case we can take advantage of LLVM's intrinsic. + const fill_byte = self.builder.buildBitCast(value, u8_llvm_ty, ""); + const len = self.sliceOrArrayLenInBytes(dest_slice, ptr_ty); + _ = self.builder.buildMemSet(dest_ptr, fill_byte, len, dest_ptr_align, ptr_ty.isVolatilePtr()); + return null; } + + // non-byte-sized element. lower with a loop. something like this: + + // entry: + // ... + // %end_ptr = getelementptr %ptr, %len + // br loop + // loop: + // %it_ptr = phi body %next_ptr, entry %ptr + // %end = cmp eq %it_ptr, %end_ptr + // cond_br %end body, end + // body: + // store %it_ptr, %value + // %next_ptr = getelementptr %it_ptr, 1 + // br loop + // end: + // ... + const entry_block = self.builder.getInsertBlock(); + const loop_block = self.context.appendBasicBlock(self.llvm_func, "InlineMemsetLoop"); + const body_block = self.context.appendBasicBlock(self.llvm_func, "InlineMemsetBody"); + const end_block = self.context.appendBasicBlock(self.llvm_func, "InlineMemsetEnd"); + + const llvm_usize_ty = self.context.intType(target.cpu.arch.ptrBitWidth()); + const len = switch (ptr_ty.ptrSize()) { + .Slice => self.builder.buildExtractValue(dest_slice, 1, ""), + .One => llvm_usize_ty.constInt(ptr_ty.childType().arrayLen(), .False), + .Many, .C => unreachable, + }; + const elem_llvm_ty = try self.dg.lowerType(elem_ty); + const len_gep = [_]*llvm.Value{len}; + const end_ptr = self.builder.buildInBoundsGEP(elem_llvm_ty, dest_ptr, &len_gep, len_gep.len, ""); + _ = self.builder.buildBr(loop_block); + + self.builder.positionBuilderAtEnd(loop_block); + const it_ptr = self.builder.buildPhi(self.context.pointerType(0), ""); + const end = self.builder.buildICmp(.NE, it_ptr, end_ptr, ""); + _ = self.builder.buildCondBr(end, body_block, end_block); + + self.builder.positionBuilderAtEnd(body_block); + const store_inst = self.builder.buildStore(value, it_ptr); + store_inst.setAlignment(@min(elem_ty.abiAlignment(target), dest_ptr_align)); + const one_gep = [_]*llvm.Value{llvm_usize_ty.constInt(1, .False)}; + const next_ptr = self.builder.buildInBoundsGEP(elem_llvm_ty, it_ptr, &one_gep, one_gep.len, ""); + _ = self.builder.buildBr(loop_block); + + self.builder.positionBuilderAtEnd(end_block); + + const incoming_values: [2]*llvm.Value = .{ next_ptr, dest_ptr }; + const incoming_blocks: [2]*llvm.BasicBlock = .{ body_block, entry_block }; + it_ptr.addIncoming(&incoming_values, &incoming_blocks, 2); + return null; } diff --git a/src/print_air.zig b/src/print_air.zig index 66732f4758..b7ee4c946a 100644 --- a/src/print_air.zig +++ b/src/print_air.zig @@ -171,6 +171,7 @@ const Writer = struct { .cmp_neq_optimized, .memcpy, .memset, + .memset_safe, => try w.writeBinOp(s, inst), .is_null, diff --git a/test/behavior/basic.zig b/test/behavior/basic.zig index 28328446c4..19ef38717a 100644 --- a/test/behavior/basic.zig +++ b/test/behavior/basic.zig @@ -353,6 +353,50 @@ fn f2(x: bool) []const u8 { return (if (x) &fA else &fB)(); } +test "@memset on array pointers" { + 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; + + try testMemsetArray(); + // TODO this doesn't pass yet + // try comptime testMemsetArray(); +} + +fn testMemsetArray() !void { + { + // memset array to non-undefined, ABI size == 1 + var foo: [20]u8 = undefined; + @memset(&foo, 'A'); + try expect(foo[0] == 'A'); + try expect(foo[11] == 'A'); + try expect(foo[19] == 'A'); + + // memset array to undefined, ABI size == 1 + @setRuntimeSafety(true); + @memset(&foo, undefined); + try expect(foo[0] == 0xaa); + try expect(foo[11] == 0xaa); + try expect(foo[19] == 0xaa); + } + + { + // memset array to non-undefined, ABI size > 1 + var foo: [20]u32 = undefined; + @memset(&foo, 1234); + try expect(foo[0] == 1234); + try expect(foo[11] == 1234); + try expect(foo[19] == 1234); + + // memset array to undefined, ABI size > 1 + @setRuntimeSafety(true); + @memset(&foo, undefined); + try expect(foo[0] == 0xaaaaaaaa); + try expect(foo[11] == 0xaaaaaaaa); + try expect(foo[19] == 0xaaaaaaaa); + } +} + test "memcpy and memset intrinsics" { if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; -- cgit v1.2.3 From 0f65cc9275cde61fe20f28e4f059c8af4c63b051 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Sun, 23 Apr 2023 13:28:44 -0700 Subject: C backend: fix memset for loop lowering Previously, this code casted the array pointer to u8 pointer, but I removed that in a different commit. This commit restores the cast, but instead of hard-coding u8, it uses the destination element pointer, since memset now supports arbitrary element types. --- src/codegen/c.zig | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'src/codegen') diff --git a/src/codegen/c.zig b/src/codegen/c.zig index 2823cbef05..582b4bf086 100644 --- a/src/codegen/c.zig +++ b/src/codegen/c.zig @@ -6233,6 +6233,15 @@ fn airMemset(f: *Function, inst: Air.Inst.Index, safety: bool) !CValue { } if (elem_abi_size > 1 or dest_ty.isVolatilePtr()) { + // For the assignment in this loop, the array pointer needs to get + // casted to a regular pointer, otherwise an error like this occurs: + // error: array type 'uint32_t[20]' (aka 'unsigned int[20]') is not assignable + var elem_ptr_ty_pl: Type.Payload.ElemType = .{ + .base = .{ .tag = .c_mut_pointer }, + .data = elem_ty, + }; + const elem_ptr_ty = Type.initPayload(&elem_ptr_ty_pl.base); + const index = try f.allocLocal(inst, Type.usize); try writer.writeAll("for ("); @@ -6256,7 +6265,9 @@ fn airMemset(f: *Function, inst: Air.Inst.Index, safety: bool) !CValue { try f.writeCValue(writer, index, .Other); try writer.writeAll(" += "); try f.object.dg.renderValue(writer, Type.usize, Value.one, .Other); - try writer.writeAll(") ("); + try writer.writeAll(") (("); + try f.renderType(writer, elem_ptr_ty); + try writer.writeByte(')'); try writeSliceOrPtr(f, writer, dest_slice, dest_ty); try writer.writeAll(")["); try f.writeCValue(writer, index, .Other); -- cgit v1.2.3 From d604553ee0c32caa0632a01e263a34e31a95b2b3 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Mon, 24 Apr 2023 18:03:48 -0700 Subject: C backend: use ++ instead of += for airMemset It does the same thing but has fewer bytes in the output. --- src/codegen/c.zig | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'src/codegen') diff --git a/src/codegen/c.zig b/src/codegen/c.zig index 582b4bf086..5efe2ee1d6 100644 --- a/src/codegen/c.zig +++ b/src/codegen/c.zig @@ -6261,10 +6261,8 @@ fn airMemset(f: *Function, inst: Air.Inst.Index, safety: bool) !CValue { }, .Many, .C => unreachable, } - try writer.writeAll("; "); + try writer.writeAll("; ++"); try f.writeCValue(writer, index, .Other); - try writer.writeAll(" += "); - try f.object.dg.renderValue(writer, Type.usize, Value.one, .Other); try writer.writeAll(") (("); try f.renderType(writer, elem_ptr_ty); try writer.writeByte(')'); -- cgit v1.2.3 From 5378fdffdcc60a5273021bc9cfc5be917e87c992 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Mon, 24 Apr 2023 17:58:23 -0700 Subject: stage2: introduce store_safe AIR instruction store: The value to store may be undefined, in which case the destination memory region has undefined bytes after this instruction is evaluated. In such case ignoring this instruction is legal lowering. store_safe: Same as `store`, except if the value to store is undefined, the memory region should be filled with 0xaa bytes, and any other safety metadata such as Valgrind integrations should be notified of this memory region being undefined. --- src/Air.zig | 16 +++++++++++++-- src/Liveness.zig | 2 ++ src/Liveness/Verify.zig | 1 + src/Sema.zig | 33 ++++++++++++++++++++++--------- src/arch/aarch64/CodeGen.zig | 10 ++++++++-- src/arch/arm/CodeGen.zig | 10 ++++++++-- src/arch/riscv64/CodeGen.zig | 10 ++++++++-- src/arch/sparc64/CodeGen.zig | 10 ++++++++-- src/arch/wasm/CodeGen.zig | 21 +++++++++++++++----- src/arch/x86_64/CodeGen.zig | 10 ++++++++-- src/codegen/c.zig | 40 ++++++++++++++----------------------- src/codegen/llvm.zig | 47 +++++++++++++++++--------------------------- src/print_air.zig | 1 + 13 files changed, 131 insertions(+), 80 deletions(-) (limited to 'src/codegen') diff --git a/src/Air.zig b/src/Air.zig index df4f861027..a2b8b9bc01 100644 --- a/src/Air.zig +++ b/src/Air.zig @@ -485,7 +485,16 @@ pub const Inst = struct { /// Write a value to a pointer. LHS is pointer, RHS is value. /// Result type is always void. /// Uses the `bin_op` field. + /// The value to store may be undefined, in which case the destination + /// memory region has undefined bytes after this instruction is + /// evaluated. In such case ignoring this instruction is legal + /// lowering. store, + /// Same as `store`, except if the value to store is undefined, the + /// memory region should be filled with 0xaa bytes, and any other + /// safety metadata such as Valgrind integrations should be notified of + /// this memory region being undefined. + store_safe, /// Indicates the program counter will never get to this instruction. /// Result type is always noreturn; no instructions in a block follow this one. unreach, @@ -639,8 +648,9 @@ pub const Inst = struct { /// Result type is always void. /// Uses the `bin_op` field. LHS is the dest slice. RHS is the element value. /// The element value may be undefined, in which case the destination - /// memory region has undefined bytes after this function executes. In - /// such case ignoring this instruction is legal lowering. + /// memory region has undefined bytes after this instruction is + /// evaluated. In such case ignoring this instruction is legal + /// lowering. /// If the length is compile-time known (due to the destination being a /// pointer-to-array), then it is guaranteed to be greater than zero. memset, @@ -1242,6 +1252,7 @@ pub fn typeOfIndex(air: Air, inst: Air.Inst.Index) Type { .dbg_var_ptr, .dbg_var_val, .store, + .store_safe, .fence, .atomic_store_unordered, .atomic_store_monotonic, @@ -1423,6 +1434,7 @@ pub fn mustLower(air: Air, inst: Air.Inst.Index) bool { .ret, .ret_load, .store, + .store_safe, .unreach, .optional_payload_ptr_set, .errunion_payload_ptr_set, diff --git a/src/Liveness.zig b/src/Liveness.zig index 32ba6927a4..6990ade327 100644 --- a/src/Liveness.zig +++ b/src/Liveness.zig @@ -299,6 +299,7 @@ pub fn categorizeOperand( }, .store, + .store_safe, .atomic_store_unordered, .atomic_store_monotonic, .atomic_store_release, @@ -965,6 +966,7 @@ fn analyzeInst( .bool_and, .bool_or, .store, + .store_safe, .array_elem_val, .slice_elem_val, .ptr_elem_val, diff --git a/src/Liveness/Verify.zig b/src/Liveness/Verify.zig index 41910485ef..a55ebe52a6 100644 --- a/src/Liveness/Verify.zig +++ b/src/Liveness/Verify.zig @@ -239,6 +239,7 @@ fn verifyBody(self: *Verify, body: []const Air.Inst.Index) Error!void { .bool_and, .bool_or, .store, + .store_safe, .array_elem_val, .slice_elem_val, .ptr_elem_val, diff --git a/src/Sema.zig b/src/Sema.zig index a2d667fd78..404bbd30a5 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -2500,7 +2500,7 @@ fn coerceResultPtr( // The last one is always `store`. const trash_inst = trash_block.instructions.items[trash_block.instructions.items.len - 1]; - if (air_tags[trash_inst] != .store) { + if (air_tags[trash_inst] != .store and air_tags[trash_inst] != .store_safe) { // no store instruction is generated for zero sized types assert((try sema.typeHasOnePossibleValue(pointee_ty)) != null); } else { @@ -3524,7 +3524,7 @@ fn zirMakePtrConst(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileErro const candidate = block.instructions.items[search_index]; switch (air_tags[candidate]) { .dbg_stmt, .dbg_block_begin, .dbg_block_end => continue, - .store => break candidate, + .store, .store_safe => break candidate, else => break :ct, } }; @@ -3750,7 +3750,7 @@ fn zirResolveInferredAlloc(sema: *Sema, block: *Block, inst: Zir.Inst.Index) Com const candidate = block.instructions.items[search_index]; switch (air_tags[candidate]) { .dbg_stmt, .dbg_block_begin, .dbg_block_end => continue, - .store => break candidate, + .store, .store_safe => break candidate, else => break :ct, } }; @@ -3860,7 +3860,7 @@ fn zirResolveInferredAlloc(sema: *Sema, block: *Block, inst: Zir.Inst.Index) Com assert(replacement_block.instructions.items.len > 0); break :result sub_ptr; }, - .store => result: { + .store, .store_safe => result: { const bin_op = sema.air_instructions.items(.data)[placeholder_inst].bin_op; try sema.storePtr2(&replacement_block, src, bin_op.lhs, src, bin_op.rhs, src, .bitcast); break :result .void_value; @@ -4242,7 +4242,10 @@ fn validateUnionInit( while (block_index > 0) : (block_index -= 1) { const store_inst = block.instructions.items[block_index]; if (store_inst == field_ptr_air_inst) break; - if (air_tags[store_inst] != .store) continue; + switch (air_tags[store_inst]) { + .store, .store_safe => {}, + else => continue, + } const bin_op = air_datas[store_inst].bin_op; var lhs = bin_op.lhs; if (Air.refToIndex(lhs)) |lhs_index| { @@ -4454,7 +4457,10 @@ fn validateStructInit( struct_is_comptime = false; continue :field; } - if (air_tags[store_inst] != .store) continue; + switch (air_tags[store_inst]) { + .store, .store_safe => {}, + else => continue, + } const bin_op = air_datas[store_inst].bin_op; var lhs = bin_op.lhs; { @@ -4682,7 +4688,10 @@ fn zirValidateArrayInit( array_is_comptime = false; continue :outer; } - if (air_tags[store_inst] != .store) continue; + switch (air_tags[store_inst]) { + .store, .store_safe => {}, + else => continue, + } const bin_op = air_datas[store_inst].bin_op; var lhs = bin_op.lhs; { @@ -5025,7 +5034,12 @@ fn zirStoreNode(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!v const ptr_src: LazySrcLoc = .{ .node_offset_store_ptr = inst_data.src_node }; const operand_src: LazySrcLoc = .{ .node_offset_store_operand = inst_data.src_node }; - const air_tag: Air.Inst.Tag = if (is_ret) .ret_ptr else .store; + const air_tag: Air.Inst.Tag = if (is_ret) + .ret_ptr + else if (block.wantSafety()) + .store_safe + else + .store; return sema.storePtr2(block, src, ptr, ptr_src, operand, operand_src, air_tag); } @@ -26704,7 +26718,8 @@ fn storePtr( ptr: Air.Inst.Ref, uncasted_operand: Air.Inst.Ref, ) CompileError!void { - return sema.storePtr2(block, src, ptr, src, uncasted_operand, src, .store); + const air_tag: Air.Inst.Tag = if (block.wantSafety()) .store_safe else .store; + return sema.storePtr2(block, src, ptr, src, uncasted_operand, src, air_tag); } fn storePtr2( diff --git a/src/arch/aarch64/CodeGen.zig b/src/arch/aarch64/CodeGen.zig index 817efc32c6..948dad73b9 100644 --- a/src/arch/aarch64/CodeGen.zig +++ b/src/arch/aarch64/CodeGen.zig @@ -764,7 +764,8 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void { .ptrtoint => try self.airPtrToInt(inst), .ret => try self.airRet(inst), .ret_load => try self.airRetLoad(inst), - .store => try self.airStore(inst), + .store => try self.airStore(inst, false), + .store_safe => try self.airStore(inst, true), .struct_field_ptr=> try self.airStructFieldPtr(inst), .struct_field_val=> try self.airStructFieldVal(inst), .array_to_slice => try self.airArrayToSlice(inst), @@ -4036,7 +4037,12 @@ fn store(self: *Self, ptr: MCValue, value: MCValue, ptr_ty: Type, value_ty: Type } } -fn airStore(self: *Self, inst: Air.Inst.Index) !void { +fn airStore(self: *Self, inst: Air.Inst.Index, safety: bool) !void { + if (safety) { + // TODO if the value is undef, write 0xaa bytes to dest + } else { + // TODO if the value is undef, don't lower this instruction + } const bin_op = self.air.instructions.items(.data)[inst].bin_op; const ptr = try self.resolveInst(bin_op.lhs); const value = try self.resolveInst(bin_op.rhs); diff --git a/src/arch/arm/CodeGen.zig b/src/arch/arm/CodeGen.zig index bdbe645878..3676b2a865 100644 --- a/src/arch/arm/CodeGen.zig +++ b/src/arch/arm/CodeGen.zig @@ -748,7 +748,8 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void { .ptrtoint => try self.airPtrToInt(inst), .ret => try self.airRet(inst), .ret_load => try self.airRetLoad(inst), - .store => try self.airStore(inst), + .store => try self.airStore(inst, false), + .store_safe => try self.airStore(inst, true), .struct_field_ptr=> try self.airStructFieldPtr(inst), .struct_field_val=> try self.airStructFieldVal(inst), .array_to_slice => try self.airArrayToSlice(inst), @@ -2836,7 +2837,12 @@ fn store(self: *Self, ptr: MCValue, value: MCValue, ptr_ty: Type, value_ty: Type } } -fn airStore(self: *Self, inst: Air.Inst.Index) !void { +fn airStore(self: *Self, inst: Air.Inst.Index, safety: bool) !void { + if (safety) { + // TODO if the value is undef, write 0xaa bytes to dest + } else { + // TODO if the value is undef, don't lower this instruction + } const bin_op = self.air.instructions.items(.data)[inst].bin_op; const ptr = try self.resolveInst(bin_op.lhs); const value = try self.resolveInst(bin_op.rhs); diff --git a/src/arch/riscv64/CodeGen.zig b/src/arch/riscv64/CodeGen.zig index 53063fa1dc..a0ebc1becc 100644 --- a/src/arch/riscv64/CodeGen.zig +++ b/src/arch/riscv64/CodeGen.zig @@ -578,7 +578,8 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void { .ptrtoint => try self.airPtrToInt(inst), .ret => try self.airRet(inst), .ret_load => try self.airRetLoad(inst), - .store => try self.airStore(inst), + .store => try self.airStore(inst, false), + .store_safe => try self.airStore(inst, true), .struct_field_ptr=> try self.airStructFieldPtr(inst), .struct_field_val=> try self.airStructFieldVal(inst), .array_to_slice => try self.airArrayToSlice(inst), @@ -1573,7 +1574,12 @@ fn store(self: *Self, ptr: MCValue, value: MCValue, ptr_ty: Type, value_ty: Type } } -fn airStore(self: *Self, inst: Air.Inst.Index) !void { +fn airStore(self: *Self, inst: Air.Inst.Index, safety: bool) !void { + if (safety) { + // TODO if the value is undef, write 0xaa bytes to dest + } else { + // TODO if the value is undef, don't lower this instruction + } const bin_op = self.air.instructions.items(.data)[inst].bin_op; const ptr = try self.resolveInst(bin_op.lhs); const value = try self.resolveInst(bin_op.rhs); diff --git a/src/arch/sparc64/CodeGen.zig b/src/arch/sparc64/CodeGen.zig index 53e07b2103..cc5c9e9832 100644 --- a/src/arch/sparc64/CodeGen.zig +++ b/src/arch/sparc64/CodeGen.zig @@ -593,7 +593,8 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void { .ptrtoint => try self.airPtrToInt(inst), .ret => try self.airRet(inst), .ret_load => try self.airRetLoad(inst), - .store => try self.airStore(inst), + .store => try self.airStore(inst, false), + .store_safe => try self.airStore(inst, true), .struct_field_ptr=> @panic("TODO try self.airStructFieldPtr(inst)"), .struct_field_val=> try self.airStructFieldVal(inst), .array_to_slice => try self.airArrayToSlice(inst), @@ -2407,7 +2408,12 @@ fn airSplat(self: *Self, inst: Air.Inst.Index) !void { return self.finishAir(inst, result, .{ ty_op.operand, .none, .none }); } -fn airStore(self: *Self, inst: Air.Inst.Index) !void { +fn airStore(self: *Self, inst: Air.Inst.Index, safety: bool) !void { + if (safety) { + // TODO if the value is undef, write 0xaa bytes to dest + } else { + // TODO if the value is undef, don't lower this instruction + } const bin_op = self.air.instructions.items(.data)[inst].bin_op; const ptr = try self.resolveInst(bin_op.lhs); const value = try self.resolveInst(bin_op.rhs); diff --git a/src/arch/wasm/CodeGen.zig b/src/arch/wasm/CodeGen.zig index adca406b53..09bc54243f 100644 --- a/src/arch/wasm/CodeGen.zig +++ b/src/arch/wasm/CodeGen.zig @@ -1883,8 +1883,8 @@ fn genInst(func: *CodeGen, inst: Air.Inst.Index) InnerError!void { .load => func.airLoad(inst), .loop => func.airLoop(inst), - // TODO: elide memset when writing undef without safety - .memset, .memset_safe => func.airMemset(inst), + .memset => func.airMemset(inst, false), + .memset_safe => func.airMemset(inst, true), .not => func.airNot(inst), .optional_payload => func.airOptionalPayload(inst), .optional_payload_ptr => func.airOptionalPayloadPtr(inst), @@ -1914,7 +1914,8 @@ fn genInst(func: *CodeGen, inst: Air.Inst.Index) InnerError!void { .slice_ptr => func.airSlicePtr(inst), .ptr_slice_len_ptr => func.airPtrSliceFieldPtr(inst, func.ptrSize()), .ptr_slice_ptr_ptr => func.airPtrSliceFieldPtr(inst, 0), - .store => func.airStore(inst), + .store => func.airStore(inst, false), + .store_safe => func.airStore(inst, true), .set_union_tag => func.airSetUnionTag(inst), .struct_field_ptr => func.airStructFieldPtr(inst), @@ -2222,7 +2223,12 @@ fn airAlloc(func: *CodeGen, inst: Air.Inst.Index) InnerError!void { func.finishAir(inst, value, &.{}); } -fn airStore(func: *CodeGen, inst: Air.Inst.Index) InnerError!void { +fn airStore(func: *CodeGen, inst: Air.Inst.Index, safety: bool) InnerError!void { + if (safety) { + // TODO if the value is undef, write 0xaa bytes to dest + } else { + // TODO if the value is undef, don't lower this instruction + } const bin_op = func.air.instructions.items(.data)[inst].bin_op; const lhs = try func.resolveInst(bin_op.lhs); @@ -4384,7 +4390,12 @@ fn airPtrBinOp(func: *CodeGen, inst: Air.Inst.Index, op: Op) InnerError!void { func.finishAir(inst, result, &.{ bin_op.lhs, bin_op.rhs }); } -fn airMemset(func: *CodeGen, inst: Air.Inst.Index) InnerError!void { +fn airMemset(func: *CodeGen, inst: Air.Inst.Index, safety: bool) InnerError!void { + if (safety) { + // TODO if the value is undef, write 0xaa bytes to dest + } else { + // TODO if the value is undef, don't lower this instruction + } const bin_op = func.air.instructions.items(.data)[inst].bin_op; const ptr = try func.resolveInst(bin_op.lhs); diff --git a/src/arch/x86_64/CodeGen.zig b/src/arch/x86_64/CodeGen.zig index 316322c60b..c857e65ec9 100644 --- a/src/arch/x86_64/CodeGen.zig +++ b/src/arch/x86_64/CodeGen.zig @@ -1035,7 +1035,8 @@ fn genBody(self: *Self, body: []const Air.Inst.Index) InnerError!void { .ptrtoint => try self.airPtrToInt(inst), .ret => try self.airRet(inst), .ret_load => try self.airRetLoad(inst), - .store => try self.airStore(inst), + .store => try self.airStore(inst, false), + .store_safe => try self.airStore(inst, true), .struct_field_ptr=> try self.airStructFieldPtr(inst), .struct_field_val=> try self.airStructFieldVal(inst), .array_to_slice => try self.airArrayToSlice(inst), @@ -3936,7 +3937,12 @@ fn store(self: *Self, ptr: MCValue, value: MCValue, ptr_ty: Type, value_ty: Type } } -fn airStore(self: *Self, inst: Air.Inst.Index) !void { +fn airStore(self: *Self, inst: Air.Inst.Index, safety: bool) !void { + if (safety) { + // TODO if the value is undef, write 0xaa bytes to dest + } else { + // TODO if the value is undef, don't lower this instruction + } const bin_op = self.air.instructions.items(.data)[inst].bin_op; const ptr = try self.resolveInst(bin_op.lhs); const ptr_ty = self.air.typeOf(bin_op.lhs); diff --git a/src/codegen/c.zig b/src/codegen/c.zig index 5efe2ee1d6..b60f3553a2 100644 --- a/src/codegen/c.zig +++ b/src/codegen/c.zig @@ -2914,7 +2914,8 @@ fn genBodyInner(f: *Function, body: []const Air.Inst.Index) error{ AnalysisFail, .load => try airLoad(f, inst), .ret => try airRet(f, inst, false), .ret_load => try airRet(f, inst, true), - .store => try airStore(f, inst), + .store => try airStore(f, inst, false), + .store_safe => try airStore(f, inst, true), .loop => try airLoop(f, inst), .cond_br => try airCondBr(f, inst), .br => try airBr(f, inst), @@ -3565,19 +3566,7 @@ fn airBoolToInt(f: *Function, inst: Air.Inst.Index) !CValue { return local; } -fn storeUndefined(f: *Function, lhs_child_ty: Type, dest_ptr: CValue) !CValue { - if (f.wantSafety()) { - const writer = f.object.writer(); - try writer.writeAll("memset("); - try f.writeCValue(writer, dest_ptr, .FunctionArgument); - try writer.print(", {x}, sizeof(", .{try f.fmtIntLiteral(Type.u8, Value.undef)}); - try f.renderType(writer, lhs_child_ty); - try writer.writeAll("));\n"); - } - return .none; -} - -fn airStore(f: *Function, inst: Air.Inst.Index) !CValue { +fn airStore(f: *Function, inst: Air.Inst.Index, safety: bool) !CValue { // *a = b; const bin_op = f.air.instructions.items(.data)[inst].bin_op; @@ -3588,18 +3577,19 @@ fn airStore(f: *Function, inst: Air.Inst.Index) !CValue { const ptr_val = try f.resolveInst(bin_op.lhs); const src_ty = f.air.typeOf(bin_op.rhs); - // TODO Sema should emit a different instruction when the store should - // possibly do the safety 0xaa bytes for undefined. - const src_val_is_undefined = - if (f.air.value(bin_op.rhs)) |v| v.isUndefDeep() else false; - if (src_val_is_undefined) { - if (ptr_info.host_size == 0) { - try reap(f, inst, &.{ bin_op.lhs, bin_op.rhs }); - return try storeUndefined(f, ptr_info.pointee_type, ptr_val); - } else if (!f.wantSafety()) { - try reap(f, inst, &.{ bin_op.lhs, bin_op.rhs }); - return .none; + const val_is_undef = if (f.air.value(bin_op.rhs)) |v| v.isUndefDeep() else false; + + if (val_is_undef) { + try reap(f, inst, &.{ bin_op.lhs, bin_op.rhs }); + if (safety and ptr_info.host_size == 0) { + const writer = f.object.writer(); + try writer.writeAll("memset("); + try f.writeCValue(writer, ptr_val, .FunctionArgument); + try writer.writeAll(", 0xaa, sizeof("); + try f.renderType(writer, ptr_info.pointee_type); + try writer.writeAll("));\n"); } + return .none; } const target = f.object.dg.module.getTarget(); diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index e8ed5745d6..ac105606e8 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -4649,7 +4649,8 @@ pub const FuncGen = struct { .not => try self.airNot(inst), .ret => try self.airRet(inst), .ret_load => try self.airRetLoad(inst), - .store => try self.airStore(inst), + .store => try self.airStore(inst, false), + .store_safe => try self.airStore(inst, true), .assembly => try self.airAssembly(inst), .slice_ptr => try self.airSliceField(inst, 0), .slice_len => try self.airSliceField(inst, 1), @@ -8115,48 +8116,36 @@ pub const FuncGen = struct { return buildAllocaInner(self.context, self.builder, self.llvm_func, self.di_scope != null, llvm_ty, alignment, self.dg.module.getTarget()); } - fn airStore(self: *FuncGen, inst: Air.Inst.Index) !?*llvm.Value { + fn airStore(self: *FuncGen, inst: Air.Inst.Index, safety: bool) !?*llvm.Value { const bin_op = self.air.instructions.items(.data)[inst].bin_op; const dest_ptr = try self.resolveInst(bin_op.lhs); const ptr_ty = self.air.typeOf(bin_op.lhs); const operand_ty = ptr_ty.childType(); - // TODO Sema should emit a different instruction when the store should - // possibly do the safety 0xaa bytes for undefined. const val_is_undef = if (self.air.value(bin_op.rhs)) |val| val.isUndefDeep() else false; if (val_is_undef) { - { - // TODO let's handle this in AIR rather than by having each backend - // check the optimization mode of the compilation because the plan is - // to support setting the optimization mode at finer grained scopes - // which happens in Sema. Codegen should not be aware of this logic. - // I think this comment is basically the same as the other TODO comment just - // above but I'm leaving them both here to make it look super messy and - // thereby bait contributors (or let's be honest, probably myself) into - // fixing this instead of letting it rot. - const safety = switch (self.dg.module.comp.bin_file.options.optimize_mode) { - .ReleaseSmall, .ReleaseFast => false, - .Debug, .ReleaseSafe => true, - }; - if (!safety) { - return null; - } - } + // Even if safety is disabled, we still emit a memset to undefined since it conveys + // extra information to LLVM. However, safety makes the difference between using + // 0xaa or actual undefined for the fill byte. + const u8_llvm_ty = self.context.intType(8); + const fill_byte = if (safety) + u8_llvm_ty.constInt(0xaa, .False) + else + u8_llvm_ty.getUndef(); const target = self.dg.module.getTarget(); const operand_size = operand_ty.abiSize(target); - const u8_llvm_ty = self.context.intType(8); - const fill_char = u8_llvm_ty.constInt(0xaa, .False); - const dest_ptr_align = ptr_ty.ptrAlignment(target); const usize_llvm_ty = try self.dg.lowerType(Type.usize); const len = usize_llvm_ty.constInt(operand_size, .False); - _ = self.builder.buildMemSet(dest_ptr, fill_char, len, dest_ptr_align, ptr_ty.isVolatilePtr()); - if (self.dg.module.comp.bin_file.options.valgrind) { + const dest_ptr_align = ptr_ty.ptrAlignment(target); + _ = self.builder.buildMemSet(dest_ptr, fill_byte, len, dest_ptr_align, ptr_ty.isVolatilePtr()); + if (safety and self.dg.module.comp.bin_file.options.valgrind) { self.valgrindMarkUndef(dest_ptr, len); } - } else { - const src_operand = try self.resolveInst(bin_op.rhs); - try self.store(dest_ptr, ptr_ty, src_operand, .NotAtomic); + return null; } + + const src_operand = try self.resolveInst(bin_op.rhs); + try self.store(dest_ptr, ptr_ty, src_operand, .NotAtomic); return null; } diff --git a/src/print_air.zig b/src/print_air.zig index b7ee4c946a..db3e47c0dd 100644 --- a/src/print_air.zig +++ b/src/print_air.zig @@ -140,6 +140,7 @@ const Writer = struct { .bool_and, .bool_or, .store, + .store_safe, .array_elem_val, .slice_elem_val, .ptr_elem_val, -- cgit v1.2.3 From 792bbfa301436fc0ce31bc4072b4765ba1408a55 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Mon, 24 Apr 2023 21:39:03 -0700 Subject: Sema: fix memcpy alias safety incorrect math Previously it was not multiplying by the element ABI size. Now, it uses ptr_add instructions which do math based on the element type. --- lib/std/os.zig | 2 +- src/Air.zig | 2 ++ src/Sema.zig | 27 ++++++++++++++++++++------ src/codegen/llvm.zig | 54 +++++++++++++++++++++++++++++++++------------------- 4 files changed, 58 insertions(+), 27 deletions(-) (limited to 'src/codegen') diff --git a/lib/std/os.zig b/lib/std/os.zig index a1fecd8501..5f4619613f 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -5548,7 +5548,7 @@ pub fn toPosixPath(file_path: []const u8) ![MAX_PATH_BYTES - 1:0]u8 { var path_with_null: [MAX_PATH_BYTES - 1:0]u8 = undefined; // >= rather than > to make room for the null byte if (file_path.len >= MAX_PATH_BYTES) return error.NameTooLong; - mem.copy(u8, &path_with_null, file_path); + @memcpy(path_with_null[0..file_path.len], file_path); path_with_null[file_path.len] = 0; return path_with_null; } diff --git a/src/Air.zig b/src/Air.zig index a2b8b9bc01..7ee36206f1 100644 --- a/src/Air.zig +++ b/src/Air.zig @@ -138,12 +138,14 @@ pub const Inst = struct { /// The offset is in element type units, not bytes. /// Wrapping is undefined behavior. /// The lhs is the pointer, rhs is the offset. Result type is the same as lhs. + /// The pointer may be a slice. /// Uses the `ty_pl` field. Payload is `Bin`. ptr_add, /// Subtract an offset from a pointer, returning a new pointer. /// The offset is in element type units, not bytes. /// Wrapping is undefined behavior. /// The lhs is the pointer, rhs is the offset. Result type is the same as lhs. + /// The pointer may be a slice. /// Uses the `ty_pl` field. Payload is `Bin`. ptr_sub, /// Given two operands which can be floats, integers, or vectors, returns the diff --git a/src/Sema.zig b/src/Sema.zig index 404bbd30a5..2c505561c9 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -21950,12 +21950,19 @@ fn zirMemcpy(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void new_src_ptr = try upgradeToArrayPtr(sema, block, src_ptr, len); } + if (dest_len != .none) { + // Change the src from slice to a many pointer, to avoid multiple ptr + // slice extractions in AIR instructions. + const new_src_ptr_ty = sema.typeOf(new_src_ptr); + if (new_src_ptr_ty.isSlice()) { + new_src_ptr = try sema.analyzeSlicePtr(block, src_src, new_src_ptr, new_src_ptr_ty); + } + } + try sema.requireRuntimeBlock(block, src, runtime_src); // Aliasing safety check. if (block.wantSafety()) { - const dest_int = try block.addUnOp(.ptrtoint, new_dest_ptr); - const src_int = try block.addUnOp(.ptrtoint, new_src_ptr); const len = if (len_val) |v| try sema.addConstant(Type.usize, v) else if (dest_len != .none) @@ -21963,12 +21970,20 @@ fn zirMemcpy(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!void else src_len; + // Extract raw pointer from dest slice. The AIR instructions could support them, but + // it would cause redundant machine code instructions. + const new_dest_ptr_ty = sema.typeOf(new_dest_ptr); + const raw_dest_ptr = if (new_dest_ptr_ty.isSlice()) + try sema.analyzeSlicePtr(block, dest_src, new_dest_ptr, new_dest_ptr_ty) + else + new_dest_ptr; + // ok1: dest >= src + len // ok2: src >= dest + len - const src_plus_len = try block.addBinOp(.add, src_int, len); - const dest_plus_len = try block.addBinOp(.add, dest_int, len); - const ok1 = try block.addBinOp(.cmp_gte, dest_int, src_plus_len); - const ok2 = try block.addBinOp(.cmp_gte, src_int, dest_plus_len); + const src_plus_len = try sema.analyzePtrArithmetic(block, src, new_src_ptr, len, .ptr_add, src_src, src); + const dest_plus_len = try sema.analyzePtrArithmetic(block, src, raw_dest_ptr, len, .ptr_add, dest_src, src); + const ok1 = try block.addBinOp(.cmp_gte, raw_dest_ptr, src_plus_len); + const ok2 = try block.addBinOp(.cmp_gte, new_src_ptr, dest_plus_len); const ok = try block.addBinOp(.bit_or, ok1, ok2); try sema.addSafetyCheck(block, ok, .memcpy_alias); } diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index ac105606e8..94f49e801d 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -7293,39 +7293,53 @@ pub const FuncGen = struct { fn airPtrAdd(self: *FuncGen, inst: Air.Inst.Index) !?*llvm.Value { const ty_pl = self.air.instructions.items(.data)[inst].ty_pl; const bin_op = self.air.extraData(Air.Bin, ty_pl.payload).data; - const base_ptr = try self.resolveInst(bin_op.lhs); + const ptr = try self.resolveInst(bin_op.lhs); const offset = try self.resolveInst(bin_op.rhs); const ptr_ty = self.air.typeOf(bin_op.lhs); const llvm_elem_ty = try self.dg.lowerPtrElemTy(ptr_ty.childType()); - if (ptr_ty.ptrSize() == .One) { - // It's a pointer to an array, so according to LLVM we need an extra GEP index. - const indices: [2]*llvm.Value = .{ - self.context.intType(32).constNull(), offset, - }; - return self.builder.buildInBoundsGEP(llvm_elem_ty, base_ptr, &indices, indices.len, ""); - } else { - const indices: [1]*llvm.Value = .{offset}; - return self.builder.buildInBoundsGEP(llvm_elem_ty, base_ptr, &indices, indices.len, ""); + switch (ptr_ty.ptrSize()) { + .One => { + // It's a pointer to an array, so according to LLVM we need an extra GEP index. + const indices: [2]*llvm.Value = .{ self.context.intType(32).constNull(), offset }; + return self.builder.buildInBoundsGEP(llvm_elem_ty, ptr, &indices, indices.len, ""); + }, + .C, .Many => { + const indices: [1]*llvm.Value = .{offset}; + return self.builder.buildInBoundsGEP(llvm_elem_ty, ptr, &indices, indices.len, ""); + }, + .Slice => { + const base = self.builder.buildExtractValue(ptr, 0, ""); + const indices: [1]*llvm.Value = .{offset}; + return self.builder.buildInBoundsGEP(llvm_elem_ty, base, &indices, indices.len, ""); + }, } } fn airPtrSub(self: *FuncGen, inst: Air.Inst.Index) !?*llvm.Value { const ty_pl = self.air.instructions.items(.data)[inst].ty_pl; const bin_op = self.air.extraData(Air.Bin, ty_pl.payload).data; - const base_ptr = try self.resolveInst(bin_op.lhs); + const ptr = try self.resolveInst(bin_op.lhs); const offset = try self.resolveInst(bin_op.rhs); const negative_offset = self.builder.buildNeg(offset, ""); const ptr_ty = self.air.typeOf(bin_op.lhs); const llvm_elem_ty = try self.dg.lowerPtrElemTy(ptr_ty.childType()); - if (ptr_ty.ptrSize() == .One) { - // It's a pointer to an array, so according to LLVM we need an extra GEP index. - const indices: [2]*llvm.Value = .{ - self.context.intType(32).constNull(), negative_offset, - }; - return self.builder.buildInBoundsGEP(llvm_elem_ty, base_ptr, &indices, indices.len, ""); - } else { - const indices: [1]*llvm.Value = .{negative_offset}; - return self.builder.buildInBoundsGEP(llvm_elem_ty, base_ptr, &indices, indices.len, ""); + switch (ptr_ty.ptrSize()) { + .One => { + // It's a pointer to an array, so according to LLVM we need an extra GEP index. + const indices: [2]*llvm.Value = .{ + self.context.intType(32).constNull(), negative_offset, + }; + return self.builder.buildInBoundsGEP(llvm_elem_ty, ptr, &indices, indices.len, ""); + }, + .C, .Many => { + const indices: [1]*llvm.Value = .{negative_offset}; + return self.builder.buildInBoundsGEP(llvm_elem_ty, ptr, &indices, indices.len, ""); + }, + .Slice => { + const base = self.builder.buildExtractValue(ptr, 0, ""); + const indices: [1]*llvm.Value = .{negative_offset}; + return self.builder.buildInBoundsGEP(llvm_elem_ty, base, &indices, indices.len, ""); + }, } } -- cgit v1.2.3 From badad16f88ac7e1eb84eadf76e13b4dc346d4ced Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 25 Apr 2023 11:22:15 -0700 Subject: C backend: fix lowering comparison when array ptr meets ptr Pointer comparisons were triggering `-Wcompare-distinct-pointer-types` before this fix, which adds `(void*)` casts if the lhs type and rhs type do not match pointer sizeness. --- src/codegen/c.zig | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'src/codegen') diff --git a/src/codegen/c.zig b/src/codegen/c.zig index b60f3553a2..1227b89208 100644 --- a/src/codegen/c.zig +++ b/src/codegen/c.zig @@ -3825,8 +3825,8 @@ fn airCmpOp( data: anytype, operator: std.math.CompareOperator, ) !CValue { - const operand_ty = f.air.typeOf(data.lhs); - const scalar_ty = operand_ty.scalarType(); + const lhs_ty = f.air.typeOf(data.lhs); + const scalar_ty = lhs_ty.scalarType(); const target = f.object.dg.module.getTarget(); const scalar_bits = scalar_ty.bitSize(target); @@ -3847,17 +3847,21 @@ fn airCmpOp( const rhs = try f.resolveInst(data.rhs); try reap(f, inst, &.{ data.lhs, data.rhs }); + const rhs_ty = f.air.typeOf(data.rhs); + const need_cast = lhs_ty.isSinglePointer() != rhs_ty.isSinglePointer(); const writer = f.object.writer(); const local = try f.allocLocal(inst, inst_ty); - const v = try Vectorize.start(f, inst, writer, operand_ty); + const v = try Vectorize.start(f, inst, writer, lhs_ty); try f.writeCValue(writer, local, .Other); try v.elem(f, writer); try writer.writeAll(" = "); + if (need_cast) try writer.writeAll("(void*)"); try f.writeCValue(writer, lhs, .Other); try v.elem(f, writer); try writer.writeByte(' '); try writer.writeAll(compareOperatorC(operator)); try writer.writeByte(' '); + if (need_cast) try writer.writeAll("(void*)"); try f.writeCValue(writer, rhs, .Other); try v.elem(f, writer); try writer.writeAll(";\n"); -- cgit v1.2.3