From a68e6cd3d73680282d06e2d7676f0bca3056c71b Mon Sep 17 00:00:00 2001 From: Jimmi Holst Christensen Date: Fri, 7 Jan 2022 18:57:39 +0100 Subject: [PATCH 1/6] Return Value.zero when the result of shr requires zero bits --- src/value.zig | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/value.zig b/src/value.zig index 9da9fa6307d6..c4d35ad006a6 100644 --- a/src/value.zig +++ b/src/value.zig @@ -2635,9 +2635,17 @@ pub const Value = extern union { var lhs_space: Value.BigIntSpace = undefined; const lhs_bigint = lhs.toBigInt(&lhs_space); const shift = @intCast(usize, rhs.toUnsignedInt()); + + const result_limbs = lhs_bigint.limbs.len -| (shift / (@sizeOf(std.math.big.Limb) * 8)); + if (result_limbs == 0) { + // The shift is enough to remove all the bits from the number, which means the + // result is zero. + return Value.zero; + } + const limbs = try allocator.alloc( std.math.big.Limb, - lhs_bigint.limbs.len - (shift / (@sizeOf(std.math.big.Limb) * 8)), + result_limbs, ); var result_bigint = BigIntMutable{ .limbs = limbs, From d5093b6c1395d4a60a923e1911321784ce368bda Mon Sep 17 00:00:00 2001 From: Jimmi Holst Christensen Date: Fri, 7 Jan 2022 22:09:35 +0100 Subject: [PATCH 2/6] Shift right is a noop if rhs is comptime known to be zero --- src/Sema.zig | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Sema.zig b/src/Sema.zig index de38d7f013dc..8389eed6d5c3 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -7318,8 +7318,8 @@ fn zirShr(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.Ins const lhs = sema.resolveInst(extra.lhs); const rhs = sema.resolveInst(extra.rhs); - if (try sema.resolveMaybeUndefVal(block, lhs_src, lhs)) |lhs_val| { - if (try sema.resolveMaybeUndefVal(block, rhs_src, rhs)) |rhs_val| { + if (try sema.resolveMaybeUndefVal(block, rhs_src, rhs)) |rhs_val| { + if (try sema.resolveMaybeUndefVal(block, lhs_src, lhs)) |lhs_val| { const lhs_ty = sema.typeOf(lhs); if (lhs_val.isUndef() or rhs_val.isUndef()) { return sema.addConstUndef(lhs_ty); @@ -7331,6 +7331,12 @@ fn zirShr(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air.Ins const val = try lhs_val.shr(rhs_val, sema.arena); return sema.addConstant(lhs_ty, val); } + // Even if lhs is not comptime known, we can still deduce certain things based + // on rhs. + // If rhs is 0, return lhs without doing any calculations. + else if (rhs_val.compareWithZero(.eq)) { + return lhs; + } } try sema.requireRuntimeBlock(block, src); From 9d6bef49a5fc2a5387734fa8d8cc698f0da8d5fa Mon Sep 17 00:00:00 2001 From: Jimmi Holst Christensen Date: Fri, 7 Jan 2022 18:58:40 +0100 Subject: [PATCH 3/6] Add two more resolution status' to Struct and Union resolveTypeForCodegen is called when we needed to resolve a type fully, even through pointer. This commit fully implements this, even through pointer fields on structs and unions. The function has now also been renamed to resolveTypeFully --- src/Module.zig | 42 +++++++++++++++++++++++- src/Sema.zig | 87 +++++++++++++++++++++++++++++++++++++++++--------- src/type.zig | 6 ++-- 3 files changed, 116 insertions(+), 19 deletions(-) diff --git a/src/Module.zig b/src/Module.zig index 0cbf75c735b0..8c14f080d256 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -831,6 +831,10 @@ pub const Struct = struct { have_field_types, layout_wip, have_layout, + fully_resolved_wip, + // The types and all its fields have had their layout resolved. Even through pointer, + // which `have_layout` does not ensure. + fully_resolved, }, /// If true, definitely nonzero size at runtime. If false, resolving the fields /// is necessary to determine whether it has bits at runtime. @@ -889,6 +893,22 @@ pub const Struct = struct { .have_field_types, .layout_wip, .have_layout, + .fully_resolved_wip, + .fully_resolved, + => true, + }; + } + + pub fn haveLayout(s: Struct) bool { + return switch (s.status) { + .none, + .field_types_wip, + .have_field_types, + .layout_wip, + => false, + .have_layout, + .fully_resolved_wip, + .fully_resolved, => true, }; } @@ -1003,6 +1023,10 @@ pub const Union = struct { have_field_types, layout_wip, have_layout, + fully_resolved_wip, + // The types and all its fields have had their layout resolved. Even through pointer, + // which `have_layout` does not ensure. + fully_resolved, }, pub const Field = struct { @@ -1033,6 +1057,8 @@ pub const Union = struct { .have_field_types, .layout_wip, .have_layout, + .fully_resolved_wip, + .fully_resolved, => true, }; } @@ -1102,8 +1128,22 @@ pub const Union = struct { tag_size: u64, }; + pub fn haveLayout(u: Union) bool { + return switch (u.status) { + .none, + .field_types_wip, + .have_field_types, + .layout_wip, + => false, + .have_layout, + .fully_resolved_wip, + .fully_resolved, + => true, + }; + } + pub fn getLayout(u: Union, target: Target, have_tag: bool) Layout { - assert(u.status == .have_layout); + assert(u.haveLayout()); var most_aligned_field: u32 = undefined; var most_aligned_field_size: u64 = undefined; var biggest_field: u32 = undefined; diff --git a/src/Sema.zig b/src/Sema.zig index 8389eed6d5c3..9ecac34c7778 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -4503,14 +4503,14 @@ fn analyzeCall( const arg_src = call_src; // TODO: better source location if (i < fn_params_len) { const param_ty = func_ty.fnParamType(i); - try sema.resolveTypeForCodegen(block, arg_src, param_ty); + try sema.resolveTypeFully(block, arg_src, param_ty); args[i] = try sema.coerce(block, param_ty, uncasted_arg, arg_src); } else { args[i] = uncasted_arg; } } - try sema.resolveTypeForCodegen(block, call_src, func_ty_info.return_type); + try sema.resolveTypeFully(block, call_src, func_ty_info.return_type); try sema.air_extra.ensureUnusedCapacity(gpa, @typeInfo(Air.Call).Struct.fields.len + args.len); @@ -4580,7 +4580,7 @@ fn finishGenericCall( const param_ty = new_fn_ty.fnParamType(runtime_i); const arg_src = call_src; // TODO: better source location const uncasted_arg = uncasted_args[total_i]; - try sema.resolveTypeForCodegen(block, arg_src, param_ty); + try sema.resolveTypeFully(block, arg_src, param_ty); const casted_arg = try sema.coerce(block, param_ty, uncasted_arg, arg_src); runtime_args[runtime_i] = casted_arg; runtime_i += 1; @@ -4588,7 +4588,7 @@ fn finishGenericCall( total_i += 1; } - try sema.resolveTypeForCodegen(block, call_src, new_fn_ty.fnReturnType()); + try sema.resolveTypeFully(block, call_src, new_fn_ty.fnReturnType()); } try sema.air_extra.ensureUnusedCapacity(sema.gpa, @typeInfo(Air.Call).Struct.fields.len + runtime_args_len); @@ -15228,7 +15228,7 @@ fn resolveStructLayout( .field_types_wip, .layout_wip => { return sema.fail(block, src, "struct {} depends on itself", .{ty}); }, - .have_layout => return, + .have_layout, .fully_resolved_wip, .fully_resolved => return, } struct_obj.status = .layout_wip; for (struct_obj.fields.values()) |field| { @@ -15250,7 +15250,7 @@ fn resolveUnionLayout( .field_types_wip, .layout_wip => { return sema.fail(block, src, "union {} depends on itself", .{ty}); }, - .have_layout => return, + .have_layout, .fully_resolved_wip, .fully_resolved => return, } union_obj.status = .layout_wip; for (union_obj.fields.values()) |field| { @@ -15259,7 +15259,7 @@ fn resolveUnionLayout( union_obj.status = .have_layout; } -fn resolveTypeForCodegen( +fn resolveTypeFully( sema: *Sema, block: *Block, src: LazySrcLoc, @@ -15268,20 +15268,67 @@ fn resolveTypeForCodegen( switch (ty.zigTypeTag()) { .Pointer => { const child_ty = try sema.resolveTypeFields(block, src, ty.childType()); - return resolveTypeForCodegen(sema, block, src, child_ty); + return resolveTypeFully(sema, block, src, child_ty); }, - .Struct => return resolveStructLayout(sema, block, src, ty), - .Union => return resolveUnionLayout(sema, block, src, ty), - .Array => return resolveTypeForCodegen(sema, block, src, ty.childType()), + .Struct => return resolveStructFully(sema, block, src, ty), + .Union => return resolveUnionFully(sema, block, src, ty), + .Array => return resolveTypeFully(sema, block, src, ty.childType()), .Optional => { var buf: Type.Payload.ElemType = undefined; - return resolveTypeForCodegen(sema, block, src, ty.optionalChild(&buf)); + return resolveTypeFully(sema, block, src, ty.optionalChild(&buf)); }, - .ErrorUnion => return resolveTypeForCodegen(sema, block, src, ty.errorUnionPayload()), + .ErrorUnion => return resolveTypeFully(sema, block, src, ty.errorUnionPayload()), else => {}, } } +fn resolveStructFully( + sema: *Sema, + block: *Block, + src: LazySrcLoc, + ty: Type, +) CompileError!void { + try resolveStructLayout(sema, block, src, ty); + + const resolved_ty = try sema.resolveTypeFields(block, src, ty); + const struct_obj = resolved_ty.castTag(.@"struct").?.data; + switch (struct_obj.status) { + .none, .have_field_types, .field_types_wip, .layout_wip, .have_layout => {}, + .fully_resolved_wip, .fully_resolved => return, + } + + // After we have resolve struct layout we have to go over the fields again to + // make sure pointer fields get their child types resolved as well + struct_obj.status = .fully_resolved_wip; + for (struct_obj.fields.values()) |field| { + try sema.resolveTypeFully(block, src, field.ty); + } + struct_obj.status = .fully_resolved; +} + +fn resolveUnionFully( + sema: *Sema, + block: *Block, + src: LazySrcLoc, + ty: Type, +) CompileError!void { + try resolveUnionLayout(sema, block, src, ty); + + const resolved_ty = try sema.resolveTypeFields(block, src, ty); + const union_obj = resolved_ty.cast(Type.Payload.Union).?.data; + switch (union_obj.status) { + .none, .have_field_types, .field_types_wip, .layout_wip, .have_layout => {}, + .fully_resolved_wip, .fully_resolved => return, + } + + // Same goes for unions (see comment about structs) + union_obj.status = .fully_resolved_wip; + for (union_obj.fields.values()) |field| { + try sema.resolveTypeFully(block, src, field.ty); + } + union_obj.status = .fully_resolved; +} + fn resolveTypeFields(sema: *Sema, block: *Block, src: LazySrcLoc, ty: Type) CompileError!Type { switch (ty.tag()) { .@"struct" => { @@ -15291,7 +15338,12 @@ fn resolveTypeFields(sema: *Sema, block: *Block, src: LazySrcLoc, ty: Type) Comp .field_types_wip => { return sema.fail(block, src, "struct {} depends on itself", .{ty}); }, - .have_field_types, .have_layout, .layout_wip => return ty, + .have_field_types, + .have_layout, + .layout_wip, + .fully_resolved_wip, + .fully_resolved, + => return ty, } struct_obj.status = .field_types_wip; @@ -15324,7 +15376,12 @@ fn resolveTypeFields(sema: *Sema, block: *Block, src: LazySrcLoc, ty: Type) Comp .field_types_wip => { return sema.fail(block, src, "union {} depends on itself", .{ty}); }, - .have_field_types, .have_layout, .layout_wip => return ty, + .have_field_types, + .have_layout, + .layout_wip, + .fully_resolved_wip, + .fully_resolved, + => return ty, } union_obj.status = .field_types_wip; diff --git a/src/type.zig b/src/type.zig index 4ad15f2399d4..f4561769e25d 100644 --- a/src/type.zig +++ b/src/type.zig @@ -1916,7 +1916,7 @@ pub const Type = extern union { const fields = self.structFields(); const is_packed = if (self.castTag(.@"struct")) |payload| p: { const struct_obj = payload.data; - assert(struct_obj.status == .have_layout); + assert(struct_obj.haveLayout()); break :p struct_obj.layout == .Packed; } else false; @@ -2220,7 +2220,7 @@ pub const Type = extern union { if (field_count == 0) return 0; const struct_obj = ty.castTag(.@"struct").?.data; - assert(struct_obj.status == .have_layout); + assert(struct_obj.haveLayout()); var total: u64 = 0; for (struct_obj.fields.values()) |field| { @@ -3771,7 +3771,7 @@ pub const Type = extern union { switch (ty.tag()) { .@"struct" => { const struct_obj = ty.castTag(.@"struct").?.data; - assert(struct_obj.status == .have_layout); + assert(struct_obj.haveLayout()); const is_packed = struct_obj.layout == .Packed; if (!is_packed) { var offset: u64 = 0; From f78d3b27ca1bea33c92e2f8eae84589db28c06cb Mon Sep 17 00:00:00 2001 From: Jimmi Holst Christensen Date: Fri, 7 Jan 2022 19:00:51 +0100 Subject: [PATCH 4/6] Increment `runtime_param_index` for zero sized parameters `runtime_param_index` is used to get the parameter type from `fn_type`, but this variable was not incremented for zero sized parameters, causing two zero sized parameters of different type to cause miss complication. --- src/Module.zig | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Module.zig b/src/Module.zig index 8c14f080d256..643e79320621 100644 --- a/src/Module.zig +++ b/src/Module.zig @@ -4437,6 +4437,7 @@ pub fn analyzeFnBody(mod: *Module, decl: *Decl, func: *Fn, arena: Allocator) Sem const arg = try sema.addConstant(param_type, opv); sema.inst_map.putAssumeCapacityNoClobber(inst, arg); total_param_index += 1; + runtime_param_index += 1; continue; } const ty_ref = try sema.addType(param_type); From f48f687c051602bfc0e8098cb91b43d971541271 Mon Sep 17 00:00:00 2001 From: Jimmi Holst Christensen Date: Fri, 7 Jan 2022 19:02:24 +0100 Subject: [PATCH 5/6] Fix llvmFieldIndex for zero sized fields It is possible for Zig to emit field ptr instructions to fields whos type is zero sized. In this case llvm should return a pointer which points to the next none zero sized parameter. --- src/codegen/llvm.zig | 54 ++++++++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 19 deletions(-) diff --git a/src/codegen/llvm.zig b/src/codegen/llvm.zig index b1046582faa4..98b906741d08 100644 --- a/src/codegen/llvm.zig +++ b/src/codegen/llvm.zig @@ -2647,7 +2647,7 @@ pub const FuncGen = struct { switch (struct_ty.zigTypeTag()) { .Struct => { var ptr_ty_buf: Type.Payload.Pointer = undefined; - const llvm_field_index = llvmFieldIndex(struct_ty, field_index, target, &ptr_ty_buf); + const llvm_field_index = llvmFieldIndex(struct_ty, field_index, target, &ptr_ty_buf).?; const field_ptr = self.builder.buildStructGEP(struct_llvm_val, llvm_field_index, ""); const field_ptr_ty = Type.initPayload(&ptr_ty_buf.base); return self.load(field_ptr, field_ptr_ty); @@ -4354,8 +4354,18 @@ pub const FuncGen = struct { .Struct => { const target = self.dg.module.getTarget(); var ty_buf: Type.Payload.Pointer = undefined; - const llvm_field_index = llvmFieldIndex(struct_ty, field_index, target, &ty_buf); - return self.builder.buildStructGEP(struct_ptr, llvm_field_index, ""); + if (llvmFieldIndex(struct_ty, field_index, target, &ty_buf)) |llvm_field_index| { + return self.builder.buildStructGEP(struct_ptr, llvm_field_index, ""); + } else { + // If we found no index then this means this is a zero sized field at the + // end of the struct. Treat our struct pointer as an array of two and get + // the index to the element at index `1` to get a pointer to the end of + // the struct. + const llvm_usize = try self.dg.llvmType(Type.usize); + const llvm_index = llvm_usize.constInt(1, .False); + const indices: [1]*const llvm.Value = .{llvm_index}; + return self.builder.buildInBoundsGEP(struct_ptr, &indices, indices.len, ""); + } }, .Union => return self.unionFieldPtr(inst, struct_ptr, struct_ty, field_index), else => unreachable, @@ -4750,32 +4760,37 @@ fn toLlvmCallConv(cc: std.builtin.CallingConvention, target: std.Target) llvm.Ca }; } -/// Take into account 0 bit fields. +/// Take into account 0 bit fields. Returns null if an llvm field could not be found. This only +/// happends if you want the field index of a zero sized field at the end of the struct. fn llvmFieldIndex( ty: Type, field_index: u32, target: std.Target, ptr_pl_buf: *Type.Payload.Pointer, -) c_uint { +) ?c_uint { const struct_obj = ty.castTag(.@"struct").?.data; if (struct_obj.layout != .Packed) { var llvm_field_index: c_uint = 0; for (struct_obj.fields.values()) |field, i| { - if (!field.ty.hasCodeGenBits()) continue; - - if (i == field_index) { - ptr_pl_buf.* = .{ - .data = .{ - .pointee_type = field.ty, - .@"align" = field.normalAlignment(target), - .@"addrspace" = .generic, - }, - }; - return llvm_field_index; + if (!field.ty.hasCodeGenBits()) + continue; + if (field_index > i) { + llvm_field_index += 1; + continue; } - llvm_field_index += 1; + + ptr_pl_buf.* = .{ + .data = .{ + .pointee_type = field.ty, + .@"align" = field.normalAlignment(target), + .@"addrspace" = .generic, + }, + }; + return llvm_field_index; + } else { + // We did not find an llvm field that corrispons to this zig field. + return null; } - unreachable; } // Our job here is to return the host integer field index. @@ -4784,7 +4799,8 @@ fn llvmFieldIndex( var running_bits: u16 = 0; var llvm_field_index: c_uint = 0; for (struct_obj.fields.values()) |field, i| { - if (!field.ty.hasCodeGenBits()) continue; + if (!field.ty.hasCodeGenBits()) + continue; const field_align = field.packedAlignment(); if (field_align == 0) { From 3871d5e55a6d52bacadd80163540a171e014605f Mon Sep 17 00:00:00 2001 From: Jimmi Holst Christensen Date: Fri, 7 Jan 2022 19:04:41 +0100 Subject: [PATCH 6/6] bit_shifting.zig now passes stage2 llvm backend --- test/behavior.zig | 2 +- test/behavior/bit_shifting.zig | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/test/behavior.zig b/test/behavior.zig index 572827781d3e..0d4cdcfb8768 100644 --- a/test/behavior.zig +++ b/test/behavior.zig @@ -73,6 +73,7 @@ test { _ = @import("behavior/array_llvm.zig"); _ = @import("behavior/atomics.zig"); _ = @import("behavior/basic_llvm.zig"); + _ = @import("behavior/bit_shifting.zig"); _ = @import("behavior/bugs/394.zig"); _ = @import("behavior/bugs/656.zig"); _ = @import("behavior/bugs/1277.zig"); @@ -120,7 +121,6 @@ test { _ = @import("behavior/async_fn.zig"); } _ = @import("behavior/await_struct.zig"); - _ = @import("behavior/bit_shifting.zig"); _ = @import("behavior/bitcast_stage1.zig"); _ = @import("behavior/bitreverse.zig"); _ = @import("behavior/bugs/421.zig"); diff --git a/test/behavior/bit_shifting.zig b/test/behavior/bit_shifting.zig index c1f84a99d4c8..0ac0ab965ecf 100644 --- a/test/behavior/bit_shifting.zig +++ b/test/behavior/bit_shifting.zig @@ -72,6 +72,7 @@ test "sharded table" { try testShardedTable(u0, 0, 1); } + fn testShardedTable(comptime Key: type, comptime mask_bit_count: comptime_int, comptime node_count: comptime_int) !void { const Table = ShardedTable(Key, mask_bit_count, void);