From ff699722da1f2df3e521c92cebe71c50910594d3 Mon Sep 17 00:00:00 2001 From: Cody Tapscott Date: Tue, 1 Nov 2022 09:22:31 -0700 Subject: [PATCH 1/6] stage2: Fix comptime array initialization This is a follow-up to 9dc98fba, which made comptime initialization patterns for union/struct more robust, especially when storing to comptime-known pointers (and globals). Resolves #13063. --- src/Sema.zig | 111 ++++++++++++++++------------------- test/behavior.zig | 1 + test/behavior/bugs/13063.zig | 16 +++++ 3 files changed, 68 insertions(+), 60 deletions(-) create mode 100644 test/behavior/bugs/13063.zig diff --git a/src/Sema.zig b/src/Sema.zig index 2704012b57..d2f530702f 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -4164,6 +4164,7 @@ fn validateStructInit( // We expect to see something like this in the current block AIR: // %a = field_ptr(...) // store(%a, %b) + // With an optional bitcast between the store and the field_ptr. // If %b is a comptime operand, this field is comptime. // // However, in the case of a comptime-known pointer to a struct, the @@ -4374,75 +4375,65 @@ fn zirValidateArrayInit( const elem_ptr_air_ref = sema.inst_map.get(elem_ptr).?; const elem_ptr_air_inst = Air.refToIndex(elem_ptr_air_ref).?; - // Find the block index of the elem_ptr so that we can look at the next - // instruction after it within the same block. + + // We expect to see something like this in the current block AIR: + // %a = elem_ptr(...) + // store(%a, %b) + // With an optional bitcast between the store and the elem_ptr. + // If %b is a comptime operand, this element is comptime. + // + // However, in the case of a comptime-known pointer to an array, the + // the elem_ptr instruction is missing, so we have to pattern-match + // based only on the store instructions. + // `first_block_index` needs to point to the `elem_ptr` if it exists; + // the `store` otherwise. + // + // It's also possible for there to be no store instruction, in the case + // of nested `coerce_result_ptr` instructions. If we see the `elem_ptr` + // but we have not found a `store`, treat as a runtime-known element. + // + // This is nearly identical to similar logic in `validateStructInit`. + // Possible performance enhancement: save the `block_index` between iterations // of the for loop. var block_index = block.instructions.items.len - 1; - while (block.instructions.items[block_index] != elem_ptr_air_inst) { - if (block_index == 0) { + while (block_index > 0) : (block_index -= 1) { + const store_inst = block.instructions.items[block_index]; + if (store_inst == elem_ptr_air_inst) { array_is_comptime = false; continue :outer; } - block_index -= 1; - } - first_block_index = @min(first_block_index, block_index); - - // If the next instructon is a store with a comptime operand, this element - // is comptime. - const next_air_inst = block.instructions.items[block_index + 1]; - switch (air_tags[next_air_inst]) { - .store => { - const bin_op = air_datas[next_air_inst].bin_op; - var lhs = bin_op.lhs; - if (Air.refToIndex(lhs)) |lhs_index| { - if (air_tags[lhs_index] == .bitcast) { - lhs = air_datas[lhs_index].ty_op.operand; - block_index -= 1; - } + if (air_tags[store_inst] != .store) continue; + const bin_op = air_datas[store_inst].bin_op; + var lhs = bin_op.lhs; + { + const lhs_index = Air.refToIndex(lhs) orelse continue; + if (air_tags[lhs_index] == .bitcast) { + lhs = air_datas[lhs_index].ty_op.operand; + block_index -= 1; } - if (lhs != elem_ptr_air_ref) { - array_is_comptime = false; - continue; - } - if (try sema.resolveMaybeUndefValAllowVariablesMaybeRuntime(block, elem_src, bin_op.rhs, &make_runtime)) |val| { - element_vals[i] = val; - } else { - array_is_comptime = false; - } - continue; - }, - .bitcast => { - // %a = bitcast(*arr_ty, %array_base) - // %b = ptr_elem_ptr(%a, %index) - // %c = bitcast(*elem_ty, %b) - // %d = store(%c, %val) - if (air_datas[next_air_inst].ty_op.operand != elem_ptr_air_ref) { - array_is_comptime = false; - continue; - } - const store_inst = block.instructions.items[block_index + 2]; - if (air_tags[store_inst] != .store) { - array_is_comptime = false; - continue; - } - const bin_op = air_datas[store_inst].bin_op; - if (bin_op.lhs != Air.indexToRef(next_air_inst)) { - array_is_comptime = false; - continue; - } - if (try sema.resolveMaybeUndefValAllowVariablesMaybeRuntime(block, elem_src, bin_op.rhs, &make_runtime)) |val| { - element_vals[i] = val; - } else { - array_is_comptime = false; - } - continue; - }, - else => { + } + if (lhs != elem_ptr_air_ref) continue; + while (block_index > 0) : (block_index -= 1) { + const block_inst = block.instructions.items[block_index - 1]; + if (air_tags[block_inst] != .dbg_stmt) break; + } + if (block_index > 0 and + elem_ptr_air_inst == block.instructions.items[block_index - 1]) + { + first_block_index = @min(first_block_index, block_index - 1); + } else { + first_block_index = @min(first_block_index, block_index); + } + if (try sema.resolveMaybeUndefValAllowVariablesMaybeRuntime(block, elem_src, bin_op.rhs, &make_runtime)) |val| { + element_vals[i] = val; + } else { array_is_comptime = false; - continue; - }, + } + continue :outer; } + array_is_comptime = false; + continue :outer; } if (array_is_comptime) { diff --git a/test/behavior.zig b/test/behavior.zig index a92a5fcbf0..41c2a79f9e 100644 --- a/test/behavior.zig +++ b/test/behavior.zig @@ -210,6 +210,7 @@ test { builtin.zig_backend != .stage2_wasm and builtin.zig_backend != .stage2_c) { + _ = @import("behavior/bugs/13063.zig"); _ = @import("behavior/bugs/11227.zig"); _ = @import("behavior/export.zig"); } diff --git a/test/behavior/bugs/13063.zig b/test/behavior/bugs/13063.zig new file mode 100644 index 0000000000..4fa0ab9d83 --- /dev/null +++ b/test/behavior/bugs/13063.zig @@ -0,0 +1,16 @@ +const std = @import("std"); +const expect = std.testing.expect; + +var pos = [2]f32{ 0.0, 0.0 }; +test "store to global array" { + try expect(pos[1] == 0.0); + pos = [2]f32{ 0.0, 1.0 }; + try expect(pos[1] == 1.0); +} + +var vpos = @Vector(2, f32){ 0.0, 0.0 }; +test "store to global vector" { + try expect(vpos[1] == 0.0); + vpos = @Vector(2, f32){ 0.0, 1.0 }; + try expect(vpos[1] == 1.0); +} From 8f3880074fb76871d9a4f35d1f72d0304ac5b404 Mon Sep 17 00:00:00 2001 From: Cody Tapscott Date: Thu, 10 Nov 2022 11:58:34 -0700 Subject: [PATCH 2/6] stage2: Be more strict about eliding loads This change makes any of the `*_val` instructions check whether it's safe to elide copies for by-ref types rather than performing this elision blindly. AIR instructions fixed: - .array_elem_val - .struct_field_val - .unwrap_errunion_payload - .try - .optional_payload These now all respect value semantics, as expected. P.S. Thanks to Andrew for the new way to approach this. Many of the lines here are from his recommended change, which comes with the significant advantage that loads are now as small as the intervening memory access allows. Co-authored by: Andrew Kelley --- src/codegen/llvm.zig | 197 +++++++++++++++++++++-------------- test/behavior.zig | 3 + test/behavior/bugs/13064.zig | 17 +++ test/behavior/bugs/13065.zig | 22 ++++ test/behavior/bugs/13069.zig | 17 +++ 5 files changed, 179 insertions(+), 77 deletions(-) create mode 100644 test/behavior/bugs/13064.zig create mode 100644 test/behavior/bugs/13065.zig create mode 100644 test/behavior/bugs/13069.zig diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index 5331862a14..b727404bfb 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -4568,14 +4568,14 @@ pub const FuncGen = struct { .ret_addr => try self.airRetAddr(inst), .frame_addr => try self.airFrameAddress(inst), .cond_br => try self.airCondBr(inst), - .@"try" => try self.airTry(inst), + .@"try" => try self.airTry(body[i..]), .try_ptr => try self.airTryPtr(inst), .intcast => try self.airIntCast(inst), .trunc => try self.airTrunc(inst), .fptrunc => try self.airFptrunc(inst), .fpext => try self.airFpext(inst), .ptrtoint => try self.airPtrToInt(inst), - .load => try self.airLoad(inst, body, i + 1), + .load => try self.airLoad(body[i..]), .loop => try self.airLoop(inst), .not => try self.airNot(inst), .ret => try self.airRet(inst), @@ -4634,7 +4634,7 @@ pub const FuncGen = struct { .atomic_store_seq_cst => try self.airAtomicStore(inst, .SequentiallyConsistent), .struct_field_ptr => try self.airStructFieldPtr(inst), - .struct_field_val => try self.airStructFieldVal(inst), + .struct_field_val => try self.airStructFieldVal(body[i..]), .struct_field_ptr_index_0 => try self.airStructFieldPtrIndex(inst, 0), .struct_field_ptr_index_1 => try self.airStructFieldPtrIndex(inst, 1), @@ -4643,18 +4643,18 @@ pub const FuncGen = struct { .field_parent_ptr => try self.airFieldParentPtr(inst), - .array_elem_val => try self.airArrayElemVal(inst), - .slice_elem_val => try self.airSliceElemVal(inst), + .array_elem_val => try self.airArrayElemVal(body[i..]), + .slice_elem_val => try self.airSliceElemVal(body[i..]), .slice_elem_ptr => try self.airSliceElemPtr(inst), - .ptr_elem_val => try self.airPtrElemVal(inst), + .ptr_elem_val => try self.airPtrElemVal(body[i..]), .ptr_elem_ptr => try self.airPtrElemPtr(inst), - .optional_payload => try self.airOptionalPayload(inst), + .optional_payload => try self.airOptionalPayload(body[i..]), .optional_payload_ptr => try self.airOptionalPayloadPtr(inst), .optional_payload_ptr_set => try self.airOptionalPayloadPtrSet(inst), - .unwrap_errunion_payload => try self.airErrUnionPayload(inst, false), - .unwrap_errunion_payload_ptr => try self.airErrUnionPayload(inst, true), + .unwrap_errunion_payload => try self.airErrUnionPayload(body[i..], false), + .unwrap_errunion_payload_ptr => try self.airErrUnionPayload(body[i..], true), .unwrap_errunion_err => try self.airErrUnionErr(inst, false), .unwrap_errunion_err_ptr => try self.airErrUnionErr(inst, true), .errunion_payload_ptr_set => try self.airErrUnionPayloadPtrSet(inst), @@ -5159,8 +5159,8 @@ pub const FuncGen = struct { _ = self.builder.buildBr(end_block); self.builder.positionBuilderAtEnd(both_pl_block); - const lhs_payload = try self.optPayloadHandle(opt_llvm_ty, lhs, scalar_ty); - const rhs_payload = try self.optPayloadHandle(opt_llvm_ty, rhs, scalar_ty); + const lhs_payload = try self.optPayloadHandle(opt_llvm_ty, lhs, scalar_ty, true); + const rhs_payload = try self.optPayloadHandle(opt_llvm_ty, rhs, scalar_ty, true); const payload_cmp = try self.cmp(lhs_payload, rhs_payload, payload_ty, op); _ = self.builder.buildBr(end_block); const both_pl_block_end = self.builder.getInsertBlock(); @@ -5305,14 +5305,16 @@ pub const FuncGen = struct { return null; } - fn airTry(self: *FuncGen, inst: Air.Inst.Index) !?*llvm.Value { + fn airTry(self: *FuncGen, body_tail: []const Air.Inst.Index) !?*llvm.Value { + const inst = body_tail[0]; const pl_op = self.air.instructions.items(.data)[inst].pl_op; const err_union = try self.resolveInst(pl_op.operand); const extra = self.air.extraData(Air.Try, pl_op.payload); const body = self.air.extra[extra.end..][0..extra.data.body_len]; const err_union_ty = self.air.typeOf(pl_op.operand); - const result_ty = self.air.typeOfIndex(inst); - return lowerTry(self, err_union, body, err_union_ty, false, result_ty); + const payload_ty = self.air.typeOfIndex(inst); + const can_elide_load = if (isByRef(payload_ty)) self.canElideLoad(body_tail) else false; + return lowerTry(self, err_union, body, err_union_ty, false, can_elide_load, payload_ty); } fn airTryPtr(self: *FuncGen, inst: Air.Inst.Index) !?*llvm.Value { @@ -5321,8 +5323,8 @@ pub const FuncGen = struct { const err_union_ptr = try self.resolveInst(extra.data.ptr); const body = self.air.extra[extra.end..][0..extra.data.body_len]; const err_union_ty = self.air.typeOf(extra.data.ptr).childType(); - const result_ty = self.air.typeOfIndex(inst); - return lowerTry(self, err_union_ptr, body, err_union_ty, true, result_ty); + const payload_ty = self.air.typeOfIndex(inst); + return lowerTry(self, err_union_ptr, body, err_union_ty, true, true, payload_ty); } fn lowerTry( @@ -5331,6 +5333,7 @@ pub const FuncGen = struct { body: []const Air.Inst.Index, err_union_ty: Type, operand_is_ptr: bool, + can_elide_load: bool, result_ty: Type, ) !?*llvm.Value { const payload_ty = err_union_ty.errorUnionPayload(); @@ -5379,12 +5382,15 @@ pub const FuncGen = struct { return fg.builder.buildBitCast(err_union, res_ptr_ty, ""); } const offset = errUnionPayloadOffset(payload_ty, target); - if (operand_is_ptr or isByRef(payload_ty)) { + if (operand_is_ptr) { return fg.builder.buildStructGEP(err_union_llvm_ty, err_union, offset, ""); } else if (isByRef(err_union_ty)) { const payload_ptr = fg.builder.buildStructGEP(err_union_llvm_ty, err_union, offset, ""); if (isByRef(payload_ty)) { - return payload_ptr; + if (can_elide_load) + return payload_ptr; + + return fg.loadByRef(payload_ptr, payload_ty, payload_ty.abiAlignment(target), false); } const load_inst = fg.builder.buildLoad(payload_ptr.getGEPResultElementType(), payload_ptr, ""); load_inst.setAlignment(payload_ty.abiAlignment(target)); @@ -5625,14 +5631,16 @@ pub const FuncGen = struct { return self.builder.buildStructGEP(slice_llvm_ty, slice_ptr, index, ""); } - fn airSliceElemVal(self: *FuncGen, inst: Air.Inst.Index) !?*llvm.Value { + fn airSliceElemVal(self: *FuncGen, body_tail: []const Air.Inst.Index) !?*llvm.Value { + const inst = body_tail[0]; const bin_op = self.air.instructions.items(.data)[inst].bin_op; const slice_ty = self.air.typeOf(bin_op.lhs); if (!slice_ty.isVolatilePtr() and self.liveness.isUnused(inst)) return null; const slice = try self.resolveInst(bin_op.lhs); const index = try self.resolveInst(bin_op.rhs); - const llvm_elem_ty = try self.dg.lowerPtrElemTy(slice_ty.childType()); + const elem_ty = slice_ty.childType(); + const llvm_elem_ty = try self.dg.lowerPtrElemTy(elem_ty); const base_ptr = self.builder.buildExtractValue(slice, 0, ""); const indices: [1]*llvm.Value = .{index}; const ptr = self.builder.buildInBoundsGEP(llvm_elem_ty, base_ptr, &indices, indices.len, ""); @@ -5653,7 +5661,8 @@ pub const FuncGen = struct { return self.builder.buildInBoundsGEP(llvm_elem_ty, base_ptr, &indices, indices.len, ""); } - fn airArrayElemVal(self: *FuncGen, inst: Air.Inst.Index) !?*llvm.Value { + fn airArrayElemVal(self: *FuncGen, body_tail: []const Air.Inst.Index) !?*llvm.Value { + const inst = body_tail[0]; if (self.liveness.isUnused(inst)) return null; const bin_op = self.air.instructions.items(.data)[inst].bin_op; @@ -5666,7 +5675,11 @@ pub const FuncGen = struct { const elem_ptr = self.builder.buildInBoundsGEP(array_llvm_ty, array_llvm_val, &indices, indices.len, ""); const elem_ty = array_ty.childType(); if (isByRef(elem_ty)) { - return elem_ptr; + if (canElideLoad(self, body_tail)) + return elem_ptr; + + const target = self.dg.module.getTarget(); + return self.loadByRef(elem_ptr, elem_ty, elem_ty.abiAlignment(target), false); } else { const elem_llvm_ty = try self.dg.lowerType(elem_ty); return self.builder.buildLoad(elem_llvm_ty, elem_ptr, ""); @@ -5677,12 +5690,14 @@ pub const FuncGen = struct { return self.builder.buildExtractElement(array_llvm_val, rhs, ""); } - fn airPtrElemVal(self: *FuncGen, inst: Air.Inst.Index) !?*llvm.Value { + fn airPtrElemVal(self: *FuncGen, body_tail: []const Air.Inst.Index) !?*llvm.Value { + const inst = body_tail[0]; const bin_op = self.air.instructions.items(.data)[inst].bin_op; const ptr_ty = self.air.typeOf(bin_op.lhs); if (!ptr_ty.isVolatilePtr() and self.liveness.isUnused(inst)) return null; - const llvm_elem_ty = try self.dg.lowerPtrElemTy(ptr_ty.childType()); + const elem_ty = ptr_ty.childType(); + const llvm_elem_ty = try self.dg.lowerPtrElemTy(elem_ty); const base_ptr = try self.resolveInst(bin_op.lhs); const rhs = try self.resolveInst(bin_op.rhs); // TODO: when we go fully opaque pointers in LLVM 16 we can remove this branch @@ -5743,7 +5758,8 @@ pub const FuncGen = struct { return self.fieldPtr(inst, struct_ptr, struct_ptr_ty, field_index); } - fn airStructFieldVal(self: *FuncGen, inst: Air.Inst.Index) !?*llvm.Value { + fn airStructFieldVal(self: *FuncGen, body_tail: []const Air.Inst.Index) !?*llvm.Value { + const inst = body_tail[0]; if (self.liveness.isUnused(inst)) return null; const ty_pl = self.air.instructions.items(.data)[inst].ty_pl; @@ -5826,7 +5842,10 @@ pub const FuncGen = struct { const llvm_field_ty = try self.dg.lowerType(field_ty); const field_ptr = self.builder.buildBitCast(union_field_ptr, llvm_field_ty.pointerType(0), ""); if (isByRef(field_ty)) { - return field_ptr; + if (canElideLoad(self, body_tail)) + return field_ptr; + + return self.loadByRef(field_ptr, field_ty, layout.payload_align, false); } else { return self.builder.buildLoad(llvm_field_ty, field_ptr, ""); } @@ -6516,7 +6535,8 @@ pub const FuncGen = struct { return self.builder.buildStructGEP(optional_llvm_ty, operand, 0, ""); } - fn airOptionalPayload(self: *FuncGen, inst: Air.Inst.Index) !?*llvm.Value { + fn airOptionalPayload(self: *FuncGen, body_tail: []const Air.Inst.Index) !?*llvm.Value { + const inst = body_tail[0]; if (self.liveness.isUnused(inst)) return null; const ty_op = self.air.instructions.items(.data)[inst].ty_op; @@ -6531,14 +6551,16 @@ pub const FuncGen = struct { } const opt_llvm_ty = try self.dg.lowerType(optional_ty); - return self.optPayloadHandle(opt_llvm_ty, operand, optional_ty); + const can_elide_load = if (isByRef(payload_ty)) self.canElideLoad(body_tail) else false; + return self.optPayloadHandle(opt_llvm_ty, operand, optional_ty, can_elide_load); } fn airErrUnionPayload( self: *FuncGen, - inst: Air.Inst.Index, + body_tail: []const Air.Inst.Index, operand_is_ptr: bool, ) !?*llvm.Value { + const inst = body_tail[0]; if (self.liveness.isUnused(inst)) return null; const ty_op = self.air.instructions.items(.data)[inst].ty_op; @@ -6558,12 +6580,15 @@ pub const FuncGen = struct { } const offset = errUnionPayloadOffset(payload_ty, target); const err_union_llvm_ty = try self.dg.lowerType(err_union_ty); - if (operand_is_ptr or isByRef(payload_ty)) { + if (operand_is_ptr) { return self.builder.buildStructGEP(err_union_llvm_ty, operand, offset, ""); } else if (isByRef(err_union_ty)) { const payload_ptr = self.builder.buildStructGEP(err_union_llvm_ty, operand, offset, ""); if (isByRef(payload_ty)) { - return payload_ptr; + if (self.canElideLoad(body_tail)) + return payload_ptr; + + return self.loadByRef(payload_ptr, payload_ty, payload_ty.abiAlignment(target), false); } const load_inst = self.builder.buildLoad(payload_ptr.getGEPResultElementType(), payload_ptr, ""); load_inst.setAlignment(payload_ty.abiAlignment(target)); @@ -8064,35 +8089,37 @@ pub const FuncGen = struct { return null; } - fn airLoad( - self: *FuncGen, - inst: Air.Inst.Index, - body: []const Air.Inst.Index, - body_i: usize, - ) !?*llvm.Value { - const ty_op = self.air.instructions.items(.data)[inst].ty_op; - const ptr_ty = self.air.typeOf(ty_op.operand); - elide: { - const ptr_info = ptr_ty.ptrInfo().data; - if (ptr_info.@"volatile") break :elide; - if (self.liveness.isUnused(inst)) return null; - if (!isByRef(ptr_info.pointee_type)) break :elide; + /// As an optimization, we want to avoid unnecessary copies of isByRef=true + /// types. Here, we scan forward in the current block, looking to see if + /// this load dies before any side effects occur. In such case, we can + /// safely return the operand without making a copy. + /// + /// The first instruction of `body_tail` is the one whose copy we want to elide. + fn canElideLoad(fg: *FuncGen, body_tail: []const Air.Inst.Index) bool { + for (body_tail[1..]) |body_inst| { + switch (fg.liveness.categorizeOperand(fg.air, body_inst, body_tail[0])) { + .none => continue, + .write, .noret, .complex => return false, + .tomb => return true, + } + } else unreachable; + } - // It would be valid to fall back to the code below here that simply calls - // load(). However, as an optimization, we want to avoid unnecessary copies - // of isByRef=true types. Here, we scan forward in the current block, - // looking to see if this load dies before any side effects occur. - // In such case, we can safely return the operand without making a copy. - for (body[body_i..]) |body_inst| { - switch (self.liveness.categorizeOperand(self.air, body_inst, inst)) { - .none => continue, - .write, .noret, .complex => break :elide, - .tomb => return try self.resolveInst(ty_op.operand), - } - } else unreachable; + fn airLoad(fg: *FuncGen, body_tail: []const Air.Inst.Index) !?*llvm.Value { + const inst = body_tail[0]; + const ty_op = fg.air.instructions.items(.data)[inst].ty_op; + const ptr_ty = fg.air.typeOf(ty_op.operand); + const ptr_info = ptr_ty.ptrInfo().data; + const ptr = try fg.resolveInst(ty_op.operand); + + elide: { + if (ptr_info.@"volatile") break :elide; + if (fg.liveness.isUnused(inst)) return null; + if (!isByRef(ptr_info.pointee_type)) break :elide; + if (!canElideLoad(fg, body_tail)) break :elide; + return ptr; } - const ptr = try self.resolveInst(ty_op.operand); - return self.load(ptr, ptr_ty); + return fg.load(ptr, ptr_ty); } fn airBreakpoint(self: *FuncGen, inst: Air.Inst.Index) !?*llvm.Value { @@ -9412,6 +9439,7 @@ pub const FuncGen = struct { opt_llvm_ty: *llvm.Type, opt_handle: *llvm.Value, opt_ty: Type, + can_elide_load: bool, ) !*llvm.Value { var buf: Type.Payload.ElemType = undefined; const payload_ty = opt_ty.optionalChild(&buf); @@ -9420,11 +9448,14 @@ pub const FuncGen = struct { // We have a pointer and we need to return a pointer to the first field. const payload_ptr = fg.builder.buildStructGEP(opt_llvm_ty, opt_handle, 0, ""); - if (isByRef(payload_ty)) { - return payload_ptr; - } const target = fg.dg.module.getTarget(); const payload_alignment = payload_ty.abiAlignment(target); + if (isByRef(payload_ty)) { + if (can_elide_load) + return payload_ptr; + + return fg.loadByRef(payload_ptr, payload_ty, payload_alignment, false); + } const payload_llvm_ty = try fg.dg.lowerType(payload_ty); const load_inst = fg.builder.buildLoad(payload_llvm_ty, payload_ptr, ""); load_inst.setAlignment(payload_alignment); @@ -9559,6 +9590,32 @@ pub const FuncGen = struct { return self.llvmModule().getIntrinsicDeclaration(id, types.ptr, types.len); } + /// Load a by-ref type by constructing a new alloca and performing a memcpy. + fn loadByRef( + fg: *FuncGen, + ptr: *llvm.Value, + pointee_type: Type, + ptr_alignment: u32, + is_volatile: bool, + ) !*llvm.Value { + const pointee_llvm_ty = try fg.dg.lowerType(pointee_type); + const target = fg.dg.module.getTarget(); + const result_align = @max(ptr_alignment, pointee_type.abiAlignment(target)); + const result_ptr = fg.buildAlloca(pointee_llvm_ty, result_align); + const llvm_ptr_u8 = fg.context.intType(8).pointerType(0); + const llvm_usize = fg.context.intType(Type.usize.intInfo(target).bits); + const size_bytes = pointee_type.abiSize(target); + _ = fg.builder.buildMemCpy( + fg.builder.buildBitCast(result_ptr, llvm_ptr_u8, ""), + result_align, + fg.builder.buildBitCast(ptr, llvm_ptr_u8, ""), + ptr_alignment, + llvm_usize.constInt(size_bytes, .False), + is_volatile, + ); + return result_ptr; + } + /// This function always performs a copy. For isByRef=true types, it creates a new /// alloca and copies the value into it, then returns the alloca instruction. /// For isByRef=false types, it creates a load instruction and returns it. @@ -9570,24 +9627,10 @@ pub const FuncGen = struct { const ptr_alignment = info.alignment(target); const ptr_volatile = llvm.Bool.fromBool(ptr_ty.isVolatilePtr()); if (info.host_size == 0) { - const elem_llvm_ty = try self.dg.lowerType(info.pointee_type); if (isByRef(info.pointee_type)) { - const result_align = info.pointee_type.abiAlignment(target); - const max_align = @max(result_align, ptr_alignment); - const result_ptr = self.buildAlloca(elem_llvm_ty, max_align); - const llvm_ptr_u8 = self.context.intType(8).pointerType(0); - const llvm_usize = self.context.intType(Type.usize.intInfo(target).bits); - const size_bytes = info.pointee_type.abiSize(target); - _ = self.builder.buildMemCpy( - self.builder.buildBitCast(result_ptr, llvm_ptr_u8, ""), - max_align, - self.builder.buildBitCast(ptr, llvm_ptr_u8, ""), - max_align, - llvm_usize.constInt(size_bytes, .False), - info.@"volatile", - ); - return result_ptr; + return self.loadByRef(ptr, info.pointee_type, ptr_alignment, info.@"volatile"); } + const elem_llvm_ty = try self.dg.lowerType(info.pointee_type); const llvm_inst = self.builder.buildLoad(elem_llvm_ty, ptr, ""); llvm_inst.setAlignment(ptr_alignment); llvm_inst.setVolatile(ptr_volatile); diff --git a/test/behavior.zig b/test/behavior.zig index 41c2a79f9e..6ccabab1e1 100644 --- a/test/behavior.zig +++ b/test/behavior.zig @@ -104,7 +104,10 @@ test { _ = @import("behavior/bugs/12945.zig"); _ = @import("behavior/bugs/12972.zig"); _ = @import("behavior/bugs/12984.zig"); + _ = @import("behavior/bugs/13064.zig"); + _ = @import("behavior/bugs/13065.zig"); _ = @import("behavior/bugs/13068.zig"); + _ = @import("behavior/bugs/13069.zig"); _ = @import("behavior/bugs/13112.zig"); _ = @import("behavior/bugs/13128.zig"); _ = @import("behavior/bugs/13164.zig"); diff --git a/test/behavior/bugs/13064.zig b/test/behavior/bugs/13064.zig new file mode 100644 index 0000000000..928e9b2aa7 --- /dev/null +++ b/test/behavior/bugs/13064.zig @@ -0,0 +1,17 @@ +const std = @import("std"); +const builtin = @import("builtin"); +const expect = std.testing.expect; + +test { + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO + + var x: [10][10]u32 = undefined; + + x[0][1] = 0; + const a = x[0]; + x[0][1] = 15; + + try expect(a[1] == 0); +} diff --git a/test/behavior/bugs/13065.zig b/test/behavior/bugs/13065.zig new file mode 100644 index 0000000000..cad1718e0b --- /dev/null +++ b/test/behavior/bugs/13065.zig @@ -0,0 +1,22 @@ +const std = @import("std"); +const builtin = @import("builtin"); +const expect = std.testing.expect; + +const U = union(enum) { + array: [10]u32, + other: u32, +}; + +test { + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO + + var x = U{ .array = undefined }; + + x.array[1] = 0; + const a = x.array; + x.array[1] = 15; + + try expect(a[1] == 0); +} diff --git a/test/behavior/bugs/13069.zig b/test/behavior/bugs/13069.zig new file mode 100644 index 0000000000..8872f8a566 --- /dev/null +++ b/test/behavior/bugs/13069.zig @@ -0,0 +1,17 @@ +const std = @import("std"); +const builtin = @import("builtin"); +const expect = std.testing.expect; + +test { + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO + + var opt_x: ?[3]f32 = [_]f32{0.0} ** 3; + + const x = opt_x.?; + opt_x.?[0] = 15.0; + + try expect(x[0] == 0.0); +} From a2f4de1663f815ae8c202ba6a8c68b0658b7d23f Mon Sep 17 00:00:00 2001 From: Cody Tapscott Date: Thu, 10 Nov 2022 12:03:17 -0700 Subject: [PATCH 3/6] stage2 llvm: Elide more loads Adds optimizations for by-ref types to: - .struct_field_val - .slice_elem_val - .ptr_elem_val I would have expected LLVM to be able to optimize away these temporaries since we don't leak pointers to them and they are fed straight from def to use, but empirically it does not. Resolves https://github.com/ziglang/zig/issues/12713 Resolves https://github.com/ziglang/zig/issues/12638 --- src/codegen/llvm.zig | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index b727404bfb..e93152df02 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -5644,6 +5644,14 @@ pub const FuncGen = struct { const base_ptr = self.builder.buildExtractValue(slice, 0, ""); const indices: [1]*llvm.Value = .{index}; const ptr = self.builder.buildInBoundsGEP(llvm_elem_ty, base_ptr, &indices, indices.len, ""); + if (isByRef(elem_ty)) { + if (self.canElideLoad(body_tail)) + return ptr; + + const target = self.dg.module.getTarget(); + return self.loadByRef(ptr, elem_ty, elem_ty.abiAlignment(target), false); + } + return self.load(ptr, slice_ty); } @@ -5709,6 +5717,14 @@ pub const FuncGen = struct { const indices: [1]*llvm.Value = .{rhs}; break :ptr self.builder.buildInBoundsGEP(llvm_elem_ty, base_ptr, &indices, indices.len, ""); }; + if (isByRef(elem_ty)) { + if (self.canElideLoad(body_tail)) + return ptr; + + const target = self.dg.module.getTarget(); + return self.loadByRef(ptr, elem_ty, elem_ty.abiAlignment(target), false); + } + return self.load(ptr, ptr_ty); } @@ -5832,7 +5848,14 @@ pub const FuncGen = struct { const struct_llvm_ty = try self.dg.lowerType(struct_ty); const field_ptr = self.builder.buildStructGEP(struct_llvm_ty, struct_llvm_val, llvm_field_index, ""); const field_ptr_ty = Type.initPayload(&ptr_ty_buf.base); - return self.load(field_ptr, field_ptr_ty); + if (isByRef(field_ty)) { + if (canElideLoad(self, body_tail)) + return field_ptr; + + return self.loadByRef(field_ptr, field_ty, ptr_ty_buf.data.alignment(target), false); + } else { + return self.load(field_ptr, field_ptr_ty); + } }, .Union => { const union_llvm_ty = try self.dg.lowerType(struct_ty); From fbda15632dfb4ddfd931b5acc0c36a3122b7e464 Mon Sep 17 00:00:00 2001 From: Cody Tapscott Date: Wed, 5 Oct 2022 05:34:45 -0700 Subject: [PATCH 4/6] stage2 sema: Make vector constants when operating on vectors Resolves https://github.com/ziglang/zig/issues/13058 --- src/Sema.zig | 105 +++++++++++++++++++++++++++++---------- test/behavior/vector.zig | 71 ++++++++++++++++++++++++-- 2 files changed, 145 insertions(+), 31 deletions(-) diff --git a/src/Sema.zig b/src/Sema.zig index d2f530702f..ffe141e560 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -8957,9 +8957,21 @@ fn intCast( const wanted_bits = wanted_info.bits; if (wanted_bits == 0) { - const zero_inst = try sema.addConstant(sema.typeOf(operand), Value.zero); - const is_in_range = try block.addBinOp(.cmp_eq, operand, zero_inst); - try sema.addSafetyCheck(block, is_in_range, .cast_truncated_data); + const ok = if (is_vector) ok: { + const zeros = try Value.Tag.repeated.create(sema.arena, Value.zero); + const zero_inst = try sema.addConstant(sema.typeOf(operand), zeros); + const is_in_range = try block.addCmpVector(operand, zero_inst, .eq, try sema.addType(operand_ty)); + const all_in_range = try block.addInst(.{ + .tag = .reduce, + .data = .{ .reduce = .{ .operand = is_in_range, .operation = .And } }, + }); + break :ok all_in_range; + } else ok: { + const zero_inst = try sema.addConstant(sema.typeOf(operand), Value.zero); + const is_in_range = try block.addBinOp(.cmp_lte, operand, zero_inst); + break :ok is_in_range; + }; + try sema.addSafetyCheck(block, ok, .cast_truncated_data); } } @@ -12376,6 +12388,8 @@ fn zirDiv(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.Ins .override = &[_]LazySrcLoc{ lhs_src, rhs_src }, }); + const is_vector = resolved_type.zigTypeTag() == .Vector; + const casted_lhs = try sema.coerce(block, resolved_type, lhs, lhs_src); const casted_rhs = try sema.coerce(block, resolved_type, rhs, rhs_src); @@ -12439,7 +12453,10 @@ fn zirDiv(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.Ins if (maybe_lhs_val) |lhs_val| { if (!lhs_val.isUndef()) { if (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { - return sema.addConstant(resolved_type, Value.zero); + const zero_val = if (is_vector) b: { + break :b try Value.Tag.repeated.create(sema.arena, Value.zero); + } else Value.zero; + return sema.addConstant(resolved_type, zero_val); } } } @@ -12532,6 +12549,8 @@ fn zirDivExact(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai .override = &[_]LazySrcLoc{ lhs_src, rhs_src }, }); + const is_vector = resolved_type.zigTypeTag() == .Vector; + const casted_lhs = try sema.coerce(block, resolved_type, lhs, lhs_src); const casted_rhs = try sema.coerce(block, resolved_type, rhs, rhs_src); @@ -12569,7 +12588,10 @@ fn zirDivExact(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai return sema.failWithUseOfUndef(block, rhs_src); } else { if (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { - return sema.addConstant(resolved_type, Value.zero); + const zero_val = if (is_vector) b: { + break :b try Value.Tag.repeated.create(sema.arena, Value.zero); + } else Value.zero; + return sema.addConstant(resolved_type, zero_val); } } } @@ -12691,6 +12713,8 @@ fn zirDivFloor(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai .override = &[_]LazySrcLoc{ lhs_src, rhs_src }, }); + const is_vector = resolved_type.zigTypeTag() == .Vector; + const casted_lhs = try sema.coerce(block, resolved_type, lhs, lhs_src); const casted_rhs = try sema.coerce(block, resolved_type, rhs, rhs_src); @@ -12730,7 +12754,10 @@ fn zirDivFloor(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai if (maybe_lhs_val) |lhs_val| { if (!lhs_val.isUndef()) { if (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { - return sema.addConstant(resolved_type, Value.zero); + const zero_val = if (is_vector) b: { + break :b try Value.Tag.repeated.create(sema.arena, Value.zero); + } else Value.zero; + return sema.addConstant(resolved_type, zero_val); } } } @@ -12803,6 +12830,8 @@ fn zirDivTrunc(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai .override = &[_]LazySrcLoc{ lhs_src, rhs_src }, }); + const is_vector = resolved_type.zigTypeTag() == .Vector; + const casted_lhs = try sema.coerce(block, resolved_type, lhs, lhs_src); const casted_rhs = try sema.coerce(block, resolved_type, rhs, rhs_src); @@ -12842,7 +12871,10 @@ fn zirDivTrunc(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai if (maybe_lhs_val) |lhs_val| { if (!lhs_val.isUndef()) { if (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { - return sema.addConstant(resolved_type, Value.zero); + const zero_val = if (is_vector) b: { + break :b try Value.Tag.repeated.create(sema.arena, Value.zero); + } else Value.zero; + return sema.addConstant(resolved_type, zero_val); } } } @@ -13042,6 +13074,8 @@ fn zirModRem(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air. .override = &[_]LazySrcLoc{ lhs_src, rhs_src }, }); + const is_vector = resolved_type.zigTypeTag() == .Vector; + const casted_lhs = try sema.coerce(block, resolved_type, lhs, lhs_src); const casted_rhs = try sema.coerce(block, resolved_type, rhs, rhs_src); @@ -13078,7 +13112,10 @@ fn zirModRem(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air. return sema.failWithUseOfUndef(block, lhs_src); } if (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { - return sema.addConstant(resolved_type, Value.zero); + const zero_val = if (is_vector) b: { + break :b try Value.Tag.repeated.create(sema.arena, Value.zero); + } else Value.zero; + return sema.addConstant(resolved_type, zero_val); } } else if (lhs_scalar_ty.isSignedInt()) { return sema.failWithModRemNegative(block, lhs_src, lhs_ty, rhs_ty); @@ -13087,25 +13124,19 @@ fn zirModRem(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air. if (rhs_val.isUndef()) { return sema.failWithUseOfUndef(block, rhs_src); } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { - return sema.failWithDivideByZero(block, rhs_src); + switch (try rhs_val.orderAgainstZeroAdvanced(sema.kit(block, src))) { + .lt => return sema.failWithModRemNegative(block, rhs_src, lhs_ty, rhs_ty), + .eq => return sema.failWithDivideByZero(block, rhs_src), + .gt => {}, } if (maybe_lhs_val) |lhs_val| { const rem_result = try sema.intRem(block, resolved_type, lhs_val, lhs_src, rhs_val, rhs_src); // If this answer could possibly be different by doing `intMod`, // we must emit a compile error. Otherwise, it's OK. - if ((try rhs_val.compareWithZeroAdvanced(.lt, sema.kit(block, src))) != (try lhs_val.compareWithZeroAdvanced(.lt, sema.kit(block, src))) and + if ((try lhs_val.compareWithZeroAdvanced(.lt, sema.kit(block, src))) and !(try rem_result.compareWithZeroAdvanced(.eq, sema.kit(block, src)))) { - const bad_src = if (try lhs_val.compareWithZeroAdvanced(.lt, sema.kit(block, src))) - lhs_src - else - rhs_src; - return sema.failWithModRemNegative(block, bad_src, lhs_ty, rhs_ty); - } - if (try lhs_val.compareWithZeroAdvanced(.lt, sema.kit(block, src))) { - // Negative - return sema.addConstant(resolved_type, Value.zero); + return sema.failWithModRemNegative(block, lhs_src, lhs_ty, rhs_ty); } return sema.addConstant(resolved_type, rem_result); } @@ -13671,6 +13702,8 @@ fn analyzeArithmetic( .override = &[_]LazySrcLoc{ lhs_src, rhs_src }, }); + const is_vector = resolved_type.zigTypeTag() == .Vector; + const casted_lhs = try sema.coerce(block, resolved_type, lhs, lhs_src); const casted_rhs = try sema.coerce(block, resolved_type, rhs, rhs_src); @@ -13897,7 +13930,10 @@ fn analyzeArithmetic( if (maybe_lhs_val) |lhs_val| { if (!lhs_val.isUndef()) { if (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { - return sema.addConstant(resolved_type, Value.zero); + const zero_val = if (is_vector) b: { + break :b try Value.Tag.repeated.create(sema.arena, Value.zero); + } else Value.zero; + return sema.addConstant(resolved_type, zero_val); } if (try sema.compare(block, src, lhs_val, .eq, Value.one, resolved_type)) { return casted_rhs; @@ -13914,7 +13950,10 @@ fn analyzeArithmetic( } } if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { - return sema.addConstant(resolved_type, Value.zero); + const zero_val = if (is_vector) b: { + break :b try Value.Tag.repeated.create(sema.arena, Value.zero); + } else Value.zero; + return sema.addConstant(resolved_type, zero_val); } if (try sema.compare(block, src, rhs_val, .eq, Value.one, resolved_type)) { return casted_lhs; @@ -13951,7 +13990,10 @@ fn analyzeArithmetic( if (maybe_lhs_val) |lhs_val| { if (!lhs_val.isUndef()) { if (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { - return sema.addConstant(resolved_type, Value.zero); + const zero_val = if (is_vector) b: { + break :b try Value.Tag.repeated.create(sema.arena, Value.zero); + } else Value.zero; + return sema.addConstant(resolved_type, zero_val); } if (try sema.compare(block, src, lhs_val, .eq, Value.one, resolved_type)) { return casted_rhs; @@ -13964,7 +14006,10 @@ fn analyzeArithmetic( return sema.addConstUndef(resolved_type); } if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { - return sema.addConstant(resolved_type, Value.zero); + const zero_val = if (is_vector) b: { + break :b try Value.Tag.repeated.create(sema.arena, Value.zero); + } else Value.zero; + return sema.addConstant(resolved_type, zero_val); } if (try sema.compare(block, src, rhs_val, .eq, Value.one, resolved_type)) { return casted_lhs; @@ -13988,7 +14033,10 @@ fn analyzeArithmetic( if (maybe_lhs_val) |lhs_val| { if (!lhs_val.isUndef()) { if (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { - return sema.addConstant(resolved_type, Value.zero); + const zero_val = if (is_vector) b: { + break :b try Value.Tag.repeated.create(sema.arena, Value.zero); + } else Value.zero; + return sema.addConstant(resolved_type, zero_val); } if (try sema.compare(block, src, lhs_val, .eq, Value.one, resolved_type)) { return casted_rhs; @@ -14000,7 +14048,10 @@ fn analyzeArithmetic( return sema.addConstUndef(resolved_type); } if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { - return sema.addConstant(resolved_type, Value.zero); + const zero_val = if (is_vector) b: { + break :b try Value.Tag.repeated.create(sema.arena, Value.zero); + } else Value.zero; + return sema.addConstant(resolved_type, zero_val); } if (try sema.compare(block, src, rhs_val, .eq, Value.one, resolved_type)) { return casted_lhs; @@ -31735,6 +31786,8 @@ fn floatToIntScalar( /// Asserts the value is an integer, and the destination type is ComptimeInt or Int. /// Vectors are also accepted. Vector results are reduced with AND. +/// +/// If provided, `vector_index` reports the first element that failed the range check. fn intFitsInType( sema: *Sema, block: *Block, diff --git a/test/behavior/vector.zig b/test/behavior/vector.zig index 20e2202b10..1f4faae636 100644 --- a/test/behavior/vector.zig +++ b/test/behavior/vector.zig @@ -1136,11 +1136,6 @@ test "array of vectors is copied" { } test "byte vector initialized in inline function" { - if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO - if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO - if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO - if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO - const S = struct { inline fn boolx4(e0: bool, e1: bool, e2: bool, e3: bool) @Vector(4, bool) { return .{ e0, e1, e2, e3 }; @@ -1170,3 +1165,69 @@ test "byte vector initialized in inline function" { try expect(S.all(S.boolx4(true, true, true, true))); } + +test "zero divisor" { + if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + + const zeros = @Vector(2, f32){ 0.0, 0.0 }; + const ones = @Vector(2, f32){ 1.0, 1.0 }; + + const v1 = zeros / ones; + const v2 = @divExact(zeros, ones); + const v3 = @divTrunc(zeros, ones); + const v4 = @divFloor(zeros, ones); + + _ = v1[0]; + _ = v2[0]; + _ = v3[0]; + _ = v4[0]; +} + +test "zero multiplicand" { + if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + + const zeros = @Vector(2, u32){ 0.0, 0.0 }; + var ones = @Vector(2, u32){ 1.0, 1.0 }; + + _ = (ones * zeros)[0]; + _ = (zeros * zeros)[0]; + _ = (zeros * ones)[0]; + + _ = (ones *| zeros)[0]; + _ = (zeros *| zeros)[0]; + _ = (zeros *| ones)[0]; + + _ = (ones *% zeros)[0]; + _ = (zeros *% zeros)[0]; + _ = (zeros *% ones)[0]; +} + +test "@intCast to u0" { + if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + + var zeros = @Vector(2, u32){ 0, 0 }; + const casted = @intCast(@Vector(2, u0), zeros); + + _ = casted[0]; +} + +test "modRem with zero divisor" { + comptime { + var zeros = @Vector(2, u32){ 0, 0 }; + const ones = @Vector(2, u32){ 1, 1 }; + + zeros %= ones; + _ = zeros[0]; + } +} From b1357091ae0876645d75a608bd1e26f21cec7c13 Mon Sep 17 00:00:00 2001 From: Cody Tapscott Date: Wed, 5 Oct 2022 05:34:49 -0700 Subject: [PATCH 5/6] Add test case for #12043 This bug is already resolved, just want to make sure we don't lose the test case. Closes #12043 --- test/behavior.zig | 1 + test/behavior/bugs/12043.zig | 12 ++++++++++++ 2 files changed, 13 insertions(+) create mode 100644 test/behavior/bugs/12043.zig diff --git a/test/behavior.zig b/test/behavior.zig index 6ccabab1e1..208e5bee29 100644 --- a/test/behavior.zig +++ b/test/behavior.zig @@ -86,6 +86,7 @@ test { _ = @import("behavior/bugs/12003.zig"); _ = @import("behavior/bugs/12025.zig"); _ = @import("behavior/bugs/12033.zig"); + _ = @import("behavior/bugs/12043.zig"); _ = @import("behavior/bugs/12430.zig"); _ = @import("behavior/bugs/12486.zig"); _ = @import("behavior/bugs/12488.zig"); diff --git a/test/behavior/bugs/12043.zig b/test/behavior/bugs/12043.zig new file mode 100644 index 0000000000..2b0d4d062d --- /dev/null +++ b/test/behavior/bugs/12043.zig @@ -0,0 +1,12 @@ +const std = @import("std"); +const expect = std.testing.expect; + +var ok = false; +fn foo(x: anytype) void { + ok = x; +} +test { + const x = &foo; + x(true); + try expect(ok); +} From 7b978bf1e05727f15fc83ae7d2455c08833cc439 Mon Sep 17 00:00:00 2001 From: Cody Tapscott Date: Wed, 5 Oct 2022 05:34:52 -0700 Subject: [PATCH 6/6] stage2: Rename `Value.compare` to `compareAll`, etc. These functions have a very error-prone API. They are essentially `all(cmp(op, ...))` but that's not reflected in the name. This renames these functions to `compareAllAgainstZero...` etc. for clarity and fixes >20 locations where the predicate was incorrect. In the future, the scalar `compare` should probably be split off from the vector comparison. Rank-polymorphic programming is great, but a proper implementation in Zig would decouple comparison and reduction, which then needs a way to fuse ops at comptime. --- src/RangeSet.zig | 6 +- src/Sema.zig | 163 ++++++++++++++++++++++++----------------------- src/type.zig | 8 +-- src/value.zig | 20 +++--- 4 files changed, 101 insertions(+), 96 deletions(-) diff --git a/src/RangeSet.zig b/src/RangeSet.zig index 84cae34365..a5007ef7c8 100644 --- a/src/RangeSet.zig +++ b/src/RangeSet.zig @@ -35,8 +35,8 @@ pub fn add( src: SwitchProngSrc, ) !?SwitchProngSrc { for (self.ranges.items) |range| { - if (last.compare(.gte, range.first, ty, self.module) and - first.compare(.lte, range.last, ty, self.module)) + if (last.compareAll(.gte, range.first, ty, self.module) and + first.compareAll(.lte, range.last, ty, self.module)) { return range.src; // They overlap. } @@ -53,7 +53,7 @@ const LessThanContext = struct { ty: Type, module: *Module }; /// Assumes a and b do not overlap fn lessThan(ctx: LessThanContext, a: Range, b: Range) bool { - return a.first.compare(.lt, b.first, ctx.ty, ctx.module); + return a.first.compareAll(.lt, b.first, ctx.ty, ctx.module); } pub fn spans(self: *RangeSet, first: Value, last: Value, ty: Type) !bool { diff --git a/src/Sema.zig b/src/Sema.zig index ffe141e560..86cc7a3b9e 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -10333,8 +10333,8 @@ fn zirSwitchBlock(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError // Validation above ensured these will succeed. const first_tv = sema.resolveInstConst(&child_block, .unneeded, item_first, "") catch unreachable; const last_tv = sema.resolveInstConst(&child_block, .unneeded, item_last, "") catch unreachable; - if ((try sema.compare(block, src, operand_val, .gte, first_tv.val, operand_ty)) and - (try sema.compare(block, src, operand_val, .lte, last_tv.val, operand_ty))) + if ((try sema.compareAll(block, src, operand_val, .gte, first_tv.val, operand_ty)) and + (try sema.compareAll(block, src, operand_val, .lte, last_tv.val, operand_ty))) { if (is_inline) child_block.inline_case_capture = operand; if (err_set) try sema.maybeErrorUnwrapComptime(&child_block, body, operand); @@ -10482,7 +10482,7 @@ fn zirSwitchBlock(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError const item_last_ref = try sema.resolveInst(last_ref); const item_last = sema.resolveConstValue(block, .unneeded, item_last_ref, undefined) catch unreachable; - while (item.compare(.lte, item_last, operand_ty, sema.mod)) : ({ + while (item.compareAll(.lte, item_last, operand_ty, sema.mod)) : ({ // Previous validation has resolved any possible lazy values. item = try sema.intAddScalar(block, .unneeded, item, Value.one); }) { @@ -10937,7 +10937,7 @@ const RangeSetUnhandledIterator = struct { it.cur = try it.sema.intAdd(it.block, it.src, it.cur, Value.one, it.ty); } it.first = false; - if (it.cur.compare(.lt, it.ranges[it.range_i].first, it.ty, it.sema.mod)) { + if (it.cur.compareAll(.lt, it.ranges[it.range_i].first, it.ty, it.sema.mod)) { return it.cur; } it.cur = it.ranges[it.range_i].last; @@ -10946,7 +10946,7 @@ const RangeSetUnhandledIterator = struct { it.cur = try it.sema.intAdd(it.block, it.src, it.cur, Value.one, it.ty); } it.first = false; - if (it.cur.compare(.lte, it.max, it.ty, it.sema.mod)) { + if (it.cur.compareAll(.lte, it.max, it.ty, it.sema.mod)) { return it.cur; } return null; @@ -10992,7 +10992,7 @@ fn validateSwitchRange( ) CompileError!void { const first_val = (try sema.resolveSwitchItemVal(block, first_ref, src_node_offset, switch_prong_src, .first)).val; const last_val = (try sema.resolveSwitchItemVal(block, last_ref, src_node_offset, switch_prong_src, .last)).val; - if (first_val.compare(.gt, last_val, operand_ty, sema.mod)) { + if (first_val.compareAll(.gt, last_val, operand_ty, sema.mod)) { const src = switch_prong_src.resolve(sema.gpa, sema.mod.declPtr(block.src_decl), src_node_offset, .first); return sema.fail(block, src, "range start value is greater than the end value", .{}); } @@ -11456,7 +11456,7 @@ fn zirShl( return sema.addConstUndef(sema.typeOf(lhs)); } // If rhs is 0, return lhs without doing any calculations. - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try rhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { return lhs; } if (scalar_ty.zigTypeTag() != .ComptimeInt and air_tag != .shl_sat) { @@ -11500,7 +11500,7 @@ fn zirShl( if (scalar_ty.zigTypeTag() == .ComptimeInt) { break :val shifted.wrapped_result; } - if (shifted.overflowed.compareWithZero(.eq)) { + if (shifted.overflowed.compareAllWithZero(.eq)) { break :val shifted.wrapped_result; } return sema.fail(block, src, "operation caused overflow", .{}); @@ -11625,7 +11625,7 @@ fn zirShr( return sema.addConstUndef(lhs_ty); } // If rhs is 0, return lhs without doing any calculations. - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try rhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { return lhs; } if (scalar_ty.zigTypeTag() != .ComptimeInt) { @@ -11659,7 +11659,7 @@ fn zirShr( if (air_tag == .shr_exact) { // Detect if any ones would be shifted out. const truncated = try lhs_val.intTruncBitsAsValue(lhs_ty, sema.arena, .unsigned, rhs_val, target); - if (!(try truncated.compareWithZeroAdvanced(.eq, sema.kit(block, src)))) { + if (!(try truncated.compareAllWithZeroAdvanced(.eq, sema.kit(block, src)))) { return sema.fail(block, src, "exact shift shifted out 1 bits", .{}); } } @@ -12414,7 +12414,7 @@ fn zirDiv(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.Ins const lhs_val = maybe_lhs_val orelse unreachable; const rhs_val = maybe_rhs_val orelse unreachable; const rem = lhs_val.floatRem(rhs_val, resolved_type, sema.arena, target) catch unreachable; - if (rem.compareWithZero(.neq)) { + if (!rem.compareAllWithZero(.eq)) { return sema.fail(block, src, "ambiguous coercion of division operands '{s}' and '{s}'; non-zero remainder '{}'", .{ @tagName(lhs_ty.tag()), @tagName(rhs_ty.tag()), rem.fmtValue(resolved_type, sema.mod), }); @@ -12452,7 +12452,7 @@ fn zirDiv(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.Ins .Int, .ComptimeInt, .ComptimeFloat => { if (maybe_lhs_val) |lhs_val| { if (!lhs_val.isUndef()) { - if (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try lhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { const zero_val = if (is_vector) b: { break :b try Value.Tag.repeated.create(sema.arena, Value.zero); } else Value.zero; @@ -12464,7 +12464,7 @@ fn zirDiv(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.Ins if (rhs_val.isUndef()) { return sema.failWithUseOfUndef(block, rhs_src); } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (!(try rhs_val.compareAllWithZeroAdvanced(.neq, sema.kit(block, src)))) { return sema.failWithDivideByZero(block, rhs_src); } // TODO: if the RHS is one, return the LHS directly @@ -12478,7 +12478,7 @@ fn zirDiv(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.Ins if (lhs_val.isUndef()) { if (lhs_scalar_ty.isSignedInt() and rhs_scalar_ty.isSignedInt()) { if (maybe_rhs_val) |rhs_val| { - if (try sema.compare(block, src, rhs_val, .neq, Value.negative_one, resolved_type)) { + if (try sema.compareAll(block, src, rhs_val, .neq, Value.negative_one, resolved_type)) { return sema.addConstUndef(resolved_type); } } @@ -12587,7 +12587,7 @@ fn zirDivExact(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai if (lhs_val.isUndef()) { return sema.failWithUseOfUndef(block, rhs_src); } else { - if (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try lhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { const zero_val = if (is_vector) b: { break :b try Value.Tag.repeated.create(sema.arena, Value.zero); } else Value.zero; @@ -12599,7 +12599,7 @@ fn zirDivExact(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai if (rhs_val.isUndef()) { return sema.failWithUseOfUndef(block, rhs_src); } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (!(try rhs_val.compareAllWithZeroAdvanced(.neq, sema.kit(block, src)))) { return sema.failWithDivideByZero(block, rhs_src); } // TODO: if the RHS is one, return the LHS directly @@ -12608,7 +12608,7 @@ fn zirDivExact(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai if (maybe_rhs_val) |rhs_val| { if (is_int) { const modulus_val = try lhs_val.intMod(rhs_val, resolved_type, sema.arena, target); - if (modulus_val.compareWithZero(.neq)) { + if (!(modulus_val.compareAllWithZero(.eq))) { return sema.fail(block, src, "exact division produced remainder", .{}); } const res = try lhs_val.intDiv(rhs_val, resolved_type, sema.arena, target); @@ -12619,7 +12619,7 @@ fn zirDivExact(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai return sema.addConstant(resolved_type, res); } else { const modulus_val = try lhs_val.floatMod(rhs_val, resolved_type, sema.arena, target); - if (modulus_val.compareWithZero(.neq)) { + if (!(modulus_val.compareAllWithZero(.eq))) { return sema.fail(block, src, "exact division produced remainder", .{}); } return sema.addConstant( @@ -12753,7 +12753,7 @@ fn zirDivFloor(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai // If the lhs is undefined, result is undefined. if (maybe_lhs_val) |lhs_val| { if (!lhs_val.isUndef()) { - if (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try lhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { const zero_val = if (is_vector) b: { break :b try Value.Tag.repeated.create(sema.arena, Value.zero); } else Value.zero; @@ -12765,7 +12765,7 @@ fn zirDivFloor(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai if (rhs_val.isUndef()) { return sema.failWithUseOfUndef(block, rhs_src); } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (!(try rhs_val.compareAllWithZeroAdvanced(.neq, sema.kit(block, src)))) { return sema.failWithDivideByZero(block, rhs_src); } // TODO: if the RHS is one, return the LHS directly @@ -12774,7 +12774,7 @@ fn zirDivFloor(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai if (lhs_val.isUndef()) { if (lhs_scalar_ty.isSignedInt() and rhs_scalar_ty.isSignedInt()) { if (maybe_rhs_val) |rhs_val| { - if (try sema.compare(block, src, rhs_val, .neq, Value.negative_one, resolved_type)) { + if (try sema.compareAll(block, src, rhs_val, .neq, Value.negative_one, resolved_type)) { return sema.addConstUndef(resolved_type); } } @@ -12870,7 +12870,7 @@ fn zirDivTrunc(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai // If the lhs is undefined, result is undefined. if (maybe_lhs_val) |lhs_val| { if (!lhs_val.isUndef()) { - if (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try lhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { const zero_val = if (is_vector) b: { break :b try Value.Tag.repeated.create(sema.arena, Value.zero); } else Value.zero; @@ -12882,7 +12882,7 @@ fn zirDivTrunc(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai if (rhs_val.isUndef()) { return sema.failWithUseOfUndef(block, rhs_src); } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (!(try rhs_val.compareAllWithZeroAdvanced(.neq, sema.kit(block, src)))) { return sema.failWithDivideByZero(block, rhs_src); } } @@ -12890,7 +12890,7 @@ fn zirDivTrunc(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Ai if (lhs_val.isUndef()) { if (lhs_scalar_ty.isSignedInt() and rhs_scalar_ty.isSignedInt()) { if (maybe_rhs_val) |rhs_val| { - if (try sema.compare(block, src, rhs_val, .neq, Value.negative_one, resolved_type)) { + if (try sema.compareAll(block, src, rhs_val, .neq, Value.negative_one, resolved_type)) { return sema.addConstUndef(resolved_type); } } @@ -12961,12 +12961,12 @@ fn addDivIntOverflowSafety( // If the LHS is comptime-known to be not equal to the min int, // no overflow is possible. if (maybe_lhs_val) |lhs_val| { - if (!lhs_val.compare(.eq, min_int, resolved_type, mod)) return; + if (lhs_val.compareAll(.neq, min_int, resolved_type, mod)) return; } // If the RHS is comptime-known to not be equal to -1, no overflow is possible. if (maybe_rhs_val) |rhs_val| { - if (!rhs_val.compare(.eq, neg_one, resolved_type, mod)) return; + if (rhs_val.compareAll(.neq, neg_one, resolved_type, mod)) return; } var ok: Air.Inst.Ref = .none; @@ -13111,7 +13111,7 @@ fn zirModRem(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air. if (lhs_val.isUndef()) { return sema.failWithUseOfUndef(block, lhs_src); } - if (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try lhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { const zero_val = if (is_vector) b: { break :b try Value.Tag.repeated.create(sema.arena, Value.zero); } else Value.zero; @@ -13124,17 +13124,18 @@ fn zirModRem(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air. if (rhs_val.isUndef()) { return sema.failWithUseOfUndef(block, rhs_src); } - switch (try rhs_val.orderAgainstZeroAdvanced(sema.kit(block, src))) { - .lt => return sema.failWithModRemNegative(block, rhs_src, lhs_ty, rhs_ty), - .eq => return sema.failWithDivideByZero(block, rhs_src), - .gt => {}, + if (!(try rhs_val.compareAllWithZeroAdvanced(.neq, sema.kit(block, src)))) { + return sema.failWithDivideByZero(block, rhs_src); + } + if (!(try rhs_val.compareAllWithZeroAdvanced(.gte, sema.kit(block, src)))) { + return sema.failWithModRemNegative(block, rhs_src, lhs_ty, rhs_ty); } if (maybe_lhs_val) |lhs_val| { const rem_result = try sema.intRem(block, resolved_type, lhs_val, lhs_src, rhs_val, rhs_src); // If this answer could possibly be different by doing `intMod`, // we must emit a compile error. Otherwise, it's OK. - if ((try lhs_val.compareWithZeroAdvanced(.lt, sema.kit(block, src))) and - !(try rem_result.compareWithZeroAdvanced(.eq, sema.kit(block, src)))) + if (!(try lhs_val.compareAllWithZeroAdvanced(.gte, sema.kit(block, src))) and + !(try rem_result.compareAllWithZeroAdvanced(.eq, sema.kit(block, src)))) { return sema.failWithModRemNegative(block, lhs_src, lhs_ty, rhs_ty); } @@ -13152,14 +13153,14 @@ fn zirModRem(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air. if (rhs_val.isUndef()) { return sema.failWithUseOfUndef(block, rhs_src); } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (!(try rhs_val.compareAllWithZeroAdvanced(.neq, sema.kit(block, src)))) { return sema.failWithDivideByZero(block, rhs_src); } - if (try rhs_val.compareWithZeroAdvanced(.lt, sema.kit(block, src))) { + if (!(try rhs_val.compareAllWithZeroAdvanced(.gte, sema.kit(block, src)))) { return sema.failWithModRemNegative(block, rhs_src, lhs_ty, rhs_ty); } if (maybe_lhs_val) |lhs_val| { - if (lhs_val.isUndef() or (try lhs_val.compareWithZeroAdvanced(.lt, sema.kit(block, src)))) { + if (lhs_val.isUndef() or !(try lhs_val.compareAllWithZeroAdvanced(.gte, sema.kit(block, src)))) { return sema.failWithModRemNegative(block, lhs_src, lhs_ty, rhs_ty); } return sema.addConstant( @@ -13295,7 +13296,7 @@ fn zirMod(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.Ins if (rhs_val.isUndef()) { return sema.failWithUseOfUndef(block, rhs_src); } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (!(try rhs_val.compareAllWithZeroAdvanced(.neq, sema.kit(block, src)))) { return sema.failWithDivideByZero(block, rhs_src); } if (maybe_lhs_val) |lhs_val| { @@ -13314,7 +13315,7 @@ fn zirMod(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.Ins if (rhs_val.isUndef()) { return sema.failWithUseOfUndef(block, rhs_src); } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (!(try rhs_val.compareAllWithZeroAdvanced(.neq, sema.kit(block, src)))) { return sema.failWithDivideByZero(block, rhs_src); } } @@ -13398,7 +13399,7 @@ fn zirRem(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.Ins if (rhs_val.isUndef()) { return sema.failWithUseOfUndef(block, rhs_src); } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (!(try rhs_val.compareAllWithZeroAdvanced(.neq, sema.kit(block, src)))) { return sema.failWithDivideByZero(block, rhs_src); } if (maybe_lhs_val) |lhs_val| { @@ -13417,7 +13418,7 @@ fn zirRem(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.Ins if (rhs_val.isUndef()) { return sema.failWithUseOfUndef(block, rhs_src); } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (!(try rhs_val.compareAllWithZeroAdvanced(.neq, sema.kit(block, src)))) { return sema.failWithDivideByZero(block, rhs_src); } } @@ -13496,12 +13497,12 @@ fn zirOverflowArithmetic( // to the result, even if it is undefined.. // Otherwise, if either of the argument is undefined, undefined is returned. if (maybe_lhs_val) |lhs_val| { - if (!lhs_val.isUndef() and (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src)))) { + if (!lhs_val.isUndef() and (try lhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src)))) { break :result .{ .overflowed = try sema.addBool(overflowed_ty, false), .wrapped = rhs }; } } if (maybe_rhs_val) |rhs_val| { - if (!rhs_val.isUndef() and (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src)))) { + if (!rhs_val.isUndef() and (try rhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src)))) { break :result .{ .overflowed = try sema.addBool(overflowed_ty, false), .wrapped = lhs }; } } @@ -13524,7 +13525,7 @@ fn zirOverflowArithmetic( if (maybe_rhs_val) |rhs_val| { if (rhs_val.isUndef()) { break :result .{ .overflowed = try sema.addConstUndef(overflowed_ty), .wrapped = try sema.addConstUndef(dest_ty) }; - } else if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + } else if (try rhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { break :result .{ .overflowed = try sema.addBool(overflowed_ty, false), .wrapped = lhs }; } else if (maybe_lhs_val) |lhs_val| { if (lhs_val.isUndef()) { @@ -13544,9 +13545,9 @@ fn zirOverflowArithmetic( // Otherwise, if either of the arguments is undefined, both results are undefined. if (maybe_lhs_val) |lhs_val| { if (!lhs_val.isUndef()) { - if (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try lhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { break :result .{ .overflowed = try sema.addBool(overflowed_ty, false), .wrapped = lhs }; - } else if (try sema.compare(block, src, lhs_val, .eq, Value.one, dest_ty)) { + } else if (try sema.compareAll(block, src, lhs_val, .eq, Value.one, dest_ty)) { break :result .{ .overflowed = try sema.addBool(overflowed_ty, false), .wrapped = rhs }; } } @@ -13554,9 +13555,9 @@ fn zirOverflowArithmetic( if (maybe_rhs_val) |rhs_val| { if (!rhs_val.isUndef()) { - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try rhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { break :result .{ .overflowed = try sema.addBool(overflowed_ty, false), .wrapped = rhs }; - } else if (try sema.compare(block, src, rhs_val, .eq, Value.one, dest_ty)) { + } else if (try sema.compareAll(block, src, rhs_val, .eq, Value.one, dest_ty)) { break :result .{ .overflowed = try sema.addBool(overflowed_ty, false), .wrapped = lhs }; } } @@ -13580,12 +13581,12 @@ fn zirOverflowArithmetic( // If rhs is zero, the result is lhs (even if undefined) and no overflow occurred. // Oterhwise if either of the arguments is undefined, both results are undefined. if (maybe_lhs_val) |lhs_val| { - if (!lhs_val.isUndef() and (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src)))) { + if (!lhs_val.isUndef() and (try lhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src)))) { break :result .{ .overflowed = try sema.addBool(overflowed_ty, false), .wrapped = lhs }; } } if (maybe_rhs_val) |rhs_val| { - if (!rhs_val.isUndef() and (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src)))) { + if (!rhs_val.isUndef() and (try rhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src)))) { break :result .{ .overflowed = try sema.addBool(overflowed_ty, false), .wrapped = lhs }; } } @@ -13728,7 +13729,7 @@ fn analyzeArithmetic( // overflow (max_int), causing illegal behavior. // For floats: either operand being undef makes the result undef. if (maybe_lhs_val) |lhs_val| { - if (!lhs_val.isUndef() and (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src)))) { + if (!lhs_val.isUndef() and (try lhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src)))) { return casted_rhs; } } @@ -13740,7 +13741,7 @@ fn analyzeArithmetic( return sema.addConstUndef(resolved_type); } } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try rhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { return casted_lhs; } } @@ -13775,7 +13776,7 @@ fn analyzeArithmetic( // If either of the operands are zero, the other operand is returned. // If either of the operands are undefined, the result is undefined. if (maybe_lhs_val) |lhs_val| { - if (!lhs_val.isUndef() and (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src)))) { + if (!lhs_val.isUndef() and (try lhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src)))) { return casted_rhs; } } @@ -13784,7 +13785,7 @@ fn analyzeArithmetic( if (rhs_val.isUndef()) { return sema.addConstUndef(resolved_type); } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try rhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { return casted_lhs; } if (maybe_lhs_val) |lhs_val| { @@ -13800,7 +13801,7 @@ fn analyzeArithmetic( // If either of the operands are zero, then the other operand is returned. // If either of the operands are undefined, the result is undefined. if (maybe_lhs_val) |lhs_val| { - if (!lhs_val.isUndef() and (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src)))) { + if (!lhs_val.isUndef() and (try lhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src)))) { return casted_rhs; } } @@ -13808,7 +13809,7 @@ fn analyzeArithmetic( if (rhs_val.isUndef()) { return sema.addConstUndef(resolved_type); } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try rhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { return casted_lhs; } if (maybe_lhs_val) |lhs_val| { @@ -13837,7 +13838,7 @@ fn analyzeArithmetic( return sema.addConstUndef(resolved_type); } } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try rhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { return casted_lhs; } } @@ -13875,7 +13876,7 @@ fn analyzeArithmetic( if (rhs_val.isUndef()) { return sema.addConstUndef(resolved_type); } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try rhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { return casted_lhs; } } @@ -13900,7 +13901,7 @@ fn analyzeArithmetic( if (rhs_val.isUndef()) { return sema.addConstUndef(resolved_type); } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try rhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { return casted_lhs; } } @@ -13929,13 +13930,13 @@ fn analyzeArithmetic( // For floats: either operand being undef makes the result undef. if (maybe_lhs_val) |lhs_val| { if (!lhs_val.isUndef()) { - if (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try lhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { const zero_val = if (is_vector) b: { break :b try Value.Tag.repeated.create(sema.arena, Value.zero); } else Value.zero; return sema.addConstant(resolved_type, zero_val); } - if (try sema.compare(block, src, lhs_val, .eq, Value.one, resolved_type)) { + if (try sema.compareAll(block, src, lhs_val, .eq, Value.one, resolved_type)) { return casted_rhs; } } @@ -13949,13 +13950,13 @@ fn analyzeArithmetic( return sema.addConstUndef(resolved_type); } } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try rhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { const zero_val = if (is_vector) b: { break :b try Value.Tag.repeated.create(sema.arena, Value.zero); } else Value.zero; return sema.addConstant(resolved_type, zero_val); } - if (try sema.compare(block, src, rhs_val, .eq, Value.one, resolved_type)) { + if (try sema.compareAll(block, src, rhs_val, .eq, Value.one, resolved_type)) { return casted_lhs; } if (maybe_lhs_val) |lhs_val| { @@ -13989,13 +13990,13 @@ fn analyzeArithmetic( // If either of the operands are undefined, result is undefined. if (maybe_lhs_val) |lhs_val| { if (!lhs_val.isUndef()) { - if (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try lhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { const zero_val = if (is_vector) b: { break :b try Value.Tag.repeated.create(sema.arena, Value.zero); } else Value.zero; return sema.addConstant(resolved_type, zero_val); } - if (try sema.compare(block, src, lhs_val, .eq, Value.one, resolved_type)) { + if (try sema.compareAll(block, src, lhs_val, .eq, Value.one, resolved_type)) { return casted_rhs; } } @@ -14005,13 +14006,13 @@ fn analyzeArithmetic( if (rhs_val.isUndef()) { return sema.addConstUndef(resolved_type); } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try rhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { const zero_val = if (is_vector) b: { break :b try Value.Tag.repeated.create(sema.arena, Value.zero); } else Value.zero; return sema.addConstant(resolved_type, zero_val); } - if (try sema.compare(block, src, rhs_val, .eq, Value.one, resolved_type)) { + if (try sema.compareAll(block, src, rhs_val, .eq, Value.one, resolved_type)) { return casted_lhs; } if (maybe_lhs_val) |lhs_val| { @@ -14032,13 +14033,13 @@ fn analyzeArithmetic( // If either of the operands are undefined, result is undefined. if (maybe_lhs_val) |lhs_val| { if (!lhs_val.isUndef()) { - if (try lhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try lhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { const zero_val = if (is_vector) b: { break :b try Value.Tag.repeated.create(sema.arena, Value.zero); } else Value.zero; return sema.addConstant(resolved_type, zero_val); } - if (try sema.compare(block, src, lhs_val, .eq, Value.one, resolved_type)) { + if (try sema.compareAll(block, src, lhs_val, .eq, Value.one, resolved_type)) { return casted_rhs; } } @@ -14047,13 +14048,13 @@ fn analyzeArithmetic( if (rhs_val.isUndef()) { return sema.addConstUndef(resolved_type); } - if (try rhs_val.compareWithZeroAdvanced(.eq, sema.kit(block, src))) { + if (try rhs_val.compareAllWithZeroAdvanced(.eq, sema.kit(block, src))) { const zero_val = if (is_vector) b: { break :b try Value.Tag.repeated.create(sema.arena, Value.zero); } else Value.zero; return sema.addConstant(resolved_type, zero_val); } - if (try sema.compare(block, src, rhs_val, .eq, Value.one, resolved_type)) { + if (try sema.compareAll(block, src, rhs_val, .eq, Value.one, resolved_type)) { return casted_lhs; } if (maybe_lhs_val) |lhs_val| { @@ -14605,7 +14606,7 @@ fn cmpSelf( return sema.addConstant(result_ty, cmp_val); } - if (try sema.compare(block, lhs_src, lhs_val, op, rhs_val, resolved_type)) { + if (try sema.compareAll(block, lhs_src, lhs_val, op, rhs_val, resolved_type)) { return Air.Inst.Ref.bool_true; } else { return Air.Inst.Ref.bool_false; @@ -27811,7 +27812,7 @@ fn analyzeSlice( sema.arena, array_ty.arrayLenIncludingSentinel(), ); - if (try sema.compare(block, src, end_val, .gt, len_s_val, Type.usize)) { + if (!(try sema.compareAll(block, src, end_val, .lte, len_s_val, Type.usize))) { const sentinel_label: []const u8 = if (array_ty.sentinel() != null) " +1 (sentinel)" else @@ -27854,7 +27855,7 @@ fn analyzeSlice( .data = slice_val.sliceLen(mod) + @boolToInt(has_sentinel), }; const slice_len_val = Value.initPayload(&int_payload.base); - if (try sema.compare(block, src, end_val, .gt, slice_len_val, Type.usize)) { + if (!(try sema.compareAll(block, src, end_val, .lte, slice_len_val, Type.usize))) { const sentinel_label: []const u8 = if (has_sentinel) " +1 (sentinel)" else @@ -27913,7 +27914,7 @@ fn analyzeSlice( // requirement: start <= end if (try sema.resolveDefinedValue(block, end_src, end)) |end_val| { if (try sema.resolveDefinedValue(block, start_src, start)) |start_val| { - if (try sema.compare(block, src, start_val, .gt, end_val, Type.usize)) { + if (!(try sema.compareAll(block, src, start_val, .lte, end_val, Type.usize))) { return sema.fail( block, start_src, @@ -28202,11 +28203,11 @@ fn cmpNumeric( // a signed integer with mantissa bits + 1, and if there was any non-integral part of the float, // add/subtract 1. const lhs_is_signed = if (try sema.resolveDefinedValue(block, lhs_src, lhs)) |lhs_val| - (try lhs_val.compareWithZeroAdvanced(.lt, sema.kit(block, src))) + !(try lhs_val.compareAllWithZeroAdvanced(.gte, sema.kit(block, src))) else (lhs_ty.isRuntimeFloat() or lhs_ty.isSignedInt()); const rhs_is_signed = if (try sema.resolveDefinedValue(block, rhs_src, rhs)) |rhs_val| - (try rhs_val.compareWithZeroAdvanced(.lt, sema.kit(block, src))) + !(try rhs_val.compareAllWithZeroAdvanced(.gte, sema.kit(block, src))) else (rhs_ty.isRuntimeFloat() or rhs_ty.isSignedInt()); const dest_int_is_signed = lhs_is_signed or rhs_is_signed; @@ -31933,13 +31934,13 @@ fn intInRange( int_val: Value, end: usize, ) !bool { - if (try int_val.compareWithZeroAdvanced(.lt, sema.kit(block, src))) return false; + if (!(try int_val.compareAllWithZeroAdvanced(.gte, sema.kit(block, src)))) return false; var end_payload: Value.Payload.U64 = .{ .base = .{ .tag = .int_u64 }, .data = end, }; const end_val = Value.initPayload(&end_payload.base); - if (try sema.compare(block, src, int_val, .gte, end_val, tag_ty)) return false; + if (!(try sema.compareAll(block, src, int_val, .lt, end_val, tag_ty))) return false; return true; } @@ -32057,8 +32058,10 @@ fn intAddWithOverflowScalar( } /// Asserts the values are comparable. Both operands have type `ty`. -/// Vector results will be reduced with AND. -fn compare( +/// For vectors, returns true if the comparison is true for ALL elements. +/// +/// Note that `!compareAll(.eq, ...) != compareAll(.neq, ...)` +fn compareAll( sema: *Sema, block: *Block, src: LazySrcLoc, diff --git a/src/type.zig b/src/type.zig index 12c969eaec..f24c89ef6f 100644 --- a/src/type.zig +++ b/src/type.zig @@ -5463,13 +5463,13 @@ pub const Type = extern union { } const S = struct { fn fieldWithRange(int_ty: Type, int_val: Value, end: usize, m: *Module) ?usize { - if (int_val.compareWithZero(.lt)) return null; + if (int_val.compareAllWithZero(.lt)) return null; var end_payload: Value.Payload.U64 = .{ .base = .{ .tag = .int_u64 }, .data = end, }; const end_val = Value.initPayload(&end_payload.base); - if (int_val.compare(.gte, end_val, int_ty, m)) return null; + if (int_val.compareAll(.gte, end_val, int_ty, m)) return null; return @intCast(usize, int_val.toUnsignedInt(m.getTarget())); } }; @@ -6455,12 +6455,12 @@ pub const Type = extern union { if (!d.mutable and d.pointee_type.eql(Type.u8, mod)) { switch (d.size) { .Slice => { - if (sent.compareWithZero(.eq)) { + if (sent.compareAllWithZero(.eq)) { return Type.initTag(.const_slice_u8_sentinel_0); } }, .Many => { - if (sent.compareWithZero(.eq)) { + if (sent.compareAllWithZero(.eq)) { return Type.initTag(.manyptr_const_u8_sentinel_0); } }, diff --git a/src/value.zig b/src/value.zig index 792ec5068f..6b5ffe02ab 100644 --- a/src/value.zig +++ b/src/value.zig @@ -2005,8 +2005,8 @@ pub const Value = extern union { } /// Asserts the values are comparable. Both operands have type `ty`. - /// Vector results will be reduced with AND. - pub fn compare(lhs: Value, op: std.math.CompareOperator, rhs: Value, ty: Type, mod: *Module) bool { + /// For vectors, returns true if comparison is true for ALL elements. + pub fn compareAll(lhs: Value, op: std.math.CompareOperator, rhs: Value, ty: Type, mod: *Module) bool { if (ty.zigTypeTag() == .Vector) { var i: usize = 0; while (i < ty.vectorLen()) : (i += 1) { @@ -2035,21 +2035,23 @@ pub const Value = extern union { } /// Asserts the value is comparable. - /// Vector results will be reduced with AND. - pub fn compareWithZero(lhs: Value, op: std.math.CompareOperator) bool { - return compareWithZeroAdvanced(lhs, op, null) catch unreachable; + /// For vectors, returns true if comparison is true for ALL elements. + /// + /// Note that `!compareAllWithZero(.eq, ...) != compareAllWithZero(.neq, ...)` + pub fn compareAllWithZero(lhs: Value, op: std.math.CompareOperator) bool { + return compareAllWithZeroAdvanced(lhs, op, null) catch unreachable; } - pub fn compareWithZeroAdvanced( + pub fn compareAllWithZeroAdvanced( lhs: Value, op: std.math.CompareOperator, sema_kit: ?Module.WipAnalysis, ) Module.CompileError!bool { switch (lhs.tag()) { - .repeated => return lhs.castTag(.repeated).?.data.compareWithZeroAdvanced(op, sema_kit), + .repeated => return lhs.castTag(.repeated).?.data.compareAllWithZeroAdvanced(op, sema_kit), .aggregate => { for (lhs.castTag(.aggregate).?.data) |elem_val| { - if (!(try elem_val.compareWithZeroAdvanced(op, sema_kit))) return false; + if (!(try elem_val.compareAllWithZeroAdvanced(op, sema_kit))) return false; } return true; }, @@ -2982,7 +2984,7 @@ pub const Value = extern union { .int_i64, .int_big_positive, .int_big_negative, - => compareWithZero(self, .eq), + => compareAllWithZero(self, .eq), .undef => unreachable, .unreachable_value => unreachable,