authorgravatar for topolarity@tapscott.meCody Tapscott <topolarity@tapscott.me> 2022-07-18 21:10:29-07:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2022-07-19 13:33:41-04:00
logb93a38860d58135072f8a1d7f6258175aa74e0fa
tree9e030102d25fe7ec2570e7561d72ed356ca23a57
parent0e26c61499c5116ca11d1bfedc691b56b23a0751

stage2: Change optional non-null field to i8

This is a workaround for https://github.com/llvm/llvm-project/issues/56585 which causes writes to i1 in memory to be optimized to an incorrect value. Unfortunately, this does not save users from running into this bug with u1 in their own types. However, this does seem to be enough to get the behavior tests working. This resolves #11450 on my machine.

1 files changed, 45 insertions(+), 34 deletions(-)

src/codegen/llvm.zig+45-34
......@@ -1499,7 +1499,7 @@ pub const Object = struct {
14991499 break :blk fwd_decl;
15001500 };
15011501
1502 const non_null_ty = Type.bool;
1502 const non_null_ty = Type.u8;
15031503 const payload_size = child_ty.abiSize(target);
15041504 const payload_align = child_ty.abiAlignment(target);
15051505 const non_null_size = non_null_ty.abiSize(target);
......@@ -2530,16 +2530,16 @@ pub const DeclGen = struct {
25302530 var buf: Type.Payload.ElemType = undefined;
25312531 const child_ty = t.optionalChild(&buf);
25322532 if (!child_ty.hasRuntimeBitsIgnoreComptime()) {
2533 return dg.context.intType(1);
2533 return dg.context.intType(8);
25342534 }
25352535 const payload_llvm_ty = try dg.lowerType(child_ty);
25362536 if (t.optionalReprIsPayload()) {
25372537 return payload_llvm_ty;
25382538 }
25392539
2540 comptime assert(optional_layout_version == 2);
2540 comptime assert(optional_layout_version == 3);
25412541 var fields_buf: [3]*const llvm.Type = .{
2542 payload_llvm_ty, dg.context.intType(1), undefined,
2542 payload_llvm_ty, dg.context.intType(8), undefined,
25432543 };
25442544 const offset = child_ty.abiSize(target) + 1;
25452545 const abi_size = t.abiSize(target);
......@@ -3134,12 +3134,13 @@ pub const DeclGen = struct {
31343134 else => unreachable,
31353135 },
31363136 .Optional => {
3137 comptime assert(optional_layout_version == 2);
3137 comptime assert(optional_layout_version == 3);
31383138 var buf: Type.Payload.ElemType = undefined;
31393139 const payload_ty = tv.ty.optionalChild(&buf);
3140 const llvm_i1 = dg.context.intType(1);
3140
3141 const llvm_i8 = dg.context.intType(8);
31413142 const is_pl = !tv.val.isNull();
3142 const non_null_bit = if (is_pl) llvm_i1.constAllOnes() else llvm_i1.constNull();
3143 const non_null_bit = if (is_pl) llvm_i8.constInt(1, .False) else llvm_i8.constNull();
31433144 if (!payload_ty.hasRuntimeBitsIgnoreComptime()) {
31443145 return non_null_bit;
31453146 }
......@@ -4041,10 +4042,10 @@ pub const FuncGen = struct {
40414042 .cmp_vector => try self.airCmpVector(inst),
40424043 .cmp_lt_errors_len => try self.airCmpLtErrorsLen(inst),
40434044
4044 .is_non_null => try self.airIsNonNull(inst, false, false, .NE),
4045 .is_non_null_ptr => try self.airIsNonNull(inst, true , false, .NE),
4046 .is_null => try self.airIsNonNull(inst, false, true , .EQ),
4047 .is_null_ptr => try self.airIsNonNull(inst, true , true , .EQ),
4045 .is_non_null => try self.airIsNonNull(inst, false, .NE),
4046 .is_non_null_ptr => try self.airIsNonNull(inst, true , .NE),
4047 .is_null => try self.airIsNonNull(inst, false, .EQ),
4048 .is_null_ptr => try self.airIsNonNull(inst, true , .EQ),
40484049
40494050 .is_non_err => try self.airIsErr(inst, .EQ, false),
40504051 .is_non_err_ptr => try self.airIsErr(inst, .EQ, true),
......@@ -5633,7 +5634,6 @@ pub const FuncGen = struct {
56335634 self: *FuncGen,
56345635 inst: Air.Inst.Index,
56355636 operand_is_ptr: bool,
5636 invert: bool,
56375637 pred: llvm.IntPredicate,
56385638 ) !?*const llvm.Value {
56395639 if (self.liveness.isUnused(inst)) return null;
......@@ -5648,20 +5648,19 @@ pub const FuncGen = struct {
56485648 return self.builder.buildICmp(pred, loaded, optional_llvm_ty.constNull(), "");
56495649 }
56505650
5651 comptime assert(optional_layout_version == 3);
5652
56515653 var buf: Type.Payload.ElemType = undefined;
56525654 const payload_ty = optional_ty.optionalChild(&buf);
56535655 if (!payload_ty.hasRuntimeBitsIgnoreComptime()) {
56545656 const loaded = if (operand_is_ptr) self.builder.buildLoad(operand, "") else operand;
5655 if (invert) {
5656 return self.builder.buildNot(loaded, "");
5657 } else {
5658 return loaded;
5659 }
5657 const llvm_i8 = self.dg.context.intType(8);
5658 return self.builder.buildICmp(pred, loaded, llvm_i8.constNull(), "");
56605659 }
56615660
56625661 const is_by_ref = operand_is_ptr or isByRef(optional_ty);
56635662 const non_null_bit = self.optIsNonNull(operand, is_by_ref);
5664 if (invert) {
5663 if (pred == .EQ) {
56655664 return self.builder.buildNot(non_null_bit, "");
56665665 } else {
56675666 return non_null_bit;
......@@ -5740,15 +5739,17 @@ pub const FuncGen = struct {
57405739 }
57415740
57425741 fn airOptionalPayloadPtrSet(self: *FuncGen, inst: Air.Inst.Index) !?*const llvm.Value {
5742 comptime assert(optional_layout_version == 3);
5743
57435744 const ty_op = self.air.instructions.items(.data)[inst].ty_op;
57445745 const operand = try self.resolveInst(ty_op.operand);
57455746 const optional_ty = self.air.typeOf(ty_op.operand).childType();
57465747 const result_ty = self.air.getRefType(ty_op.ty);
57475748 var buf: Type.Payload.ElemType = undefined;
57485749 const payload_ty = optional_ty.optionalChild(&buf);
5749 const non_null_bit = self.context.intType(1).constAllOnes();
5750 const non_null_bit = self.context.intType(8).constInt(1, .False);
57505751 if (!payload_ty.hasRuntimeBitsIgnoreComptime()) {
5751 // We have a pointer to a i1. We need to set it to 1 and then return the same pointer.
5752 // We have a pointer to a i8. We need to set it to 1 and then return the same pointer.
57525753 _ = self.builder.buildStore(non_null_bit, operand);
57535754
57545755 // TODO once we update to LLVM 14 this bitcast won't be necessary.
......@@ -5914,8 +5915,8 @@ pub const FuncGen = struct {
59145915
59155916 const ty_op = self.air.instructions.items(.data)[inst].ty_op;
59165917 const payload_ty = self.air.typeOf(ty_op.operand);
5917 const non_null_bit = self.context.intType(1).constAllOnes();
5918 comptime assert(optional_layout_version == 2);
5918 const non_null_bit = self.context.intType(8).constInt(1, .False);
5919 comptime assert(optional_layout_version == 3);
59195920 if (!payload_ty.hasRuntimeBitsIgnoreComptime()) return non_null_bit;
59205921 const operand = try self.resolveInst(ty_op.operand);
59215922 const optional_ty = self.air.typeOfIndex(inst);
......@@ -7345,10 +7346,12 @@ pub const FuncGen = struct {
73457346 return self.builder.buildSelect(success_bit, payload.typeOf().constNull(), payload, "");
73467347 }
73477348
7349 comptime assert(optional_layout_version == 3);
73487350 const optional_llvm_ty = try self.dg.lowerType(optional_ty);
73497351 const non_null_bit = self.builder.buildNot(success_bit, "");
7352 const non_null_field = self.builder.buildZExt(non_null_bit, self.dg.context.intType(8), "");
73507353 const partial = self.builder.buildInsertValue(optional_llvm_ty.getUndef(), payload, 0, "");
7351 return self.builder.buildInsertValue(partial, non_null_bit, 1, "");
7354 return self.builder.buildInsertValue(partial, non_null_field, 1, "");
73527355 }
73537356
73547357 fn airAtomicRmw(self: *FuncGen, inst: Air.Inst.Index) !?*const llvm.Value {
......@@ -8331,19 +8334,24 @@ pub const FuncGen = struct {
83318334
83328335 /// Assumes the optional is not pointer-like and payload has bits.
83338336 fn optIsNonNull(self: *FuncGen, opt_handle: *const llvm.Value, is_by_ref: bool) *const llvm.Value {
8334 if (is_by_ref) {
8335 const index_type = self.context.intType(32);
8337 const field = b: {
8338 if (is_by_ref) {
8339 const index_type = self.context.intType(32);
83368340
8337 const indices: [2]*const llvm.Value = .{
8338 index_type.constNull(),
8339 index_type.constInt(1, .False),
8340 };
8341 const indices: [2]*const llvm.Value = .{
8342 index_type.constNull(),
8343 index_type.constInt(1, .False),
8344 };
83418345
8342 const field_ptr = self.builder.buildInBoundsGEP(opt_handle, &indices, indices.len, "");
8343 return self.builder.buildLoad(field_ptr, "");
8344 }
8346 const field_ptr = self.builder.buildInBoundsGEP(opt_handle, &indices, indices.len, "");
8347 break :b self.builder.buildLoad(field_ptr, "");
8348 }
8349
8350 break :b self.builder.buildExtractValue(opt_handle, 1, "");
8351 };
8352 comptime assert(optional_layout_version == 3);
83458353
8346 return self.builder.buildExtractValue(opt_handle, 1, "");
8354 return self.builder.buildICmp(.NE, field, self.context.intType(8).constInt(0, .False), "");
83478355 }
83488356
83498357 /// Assumes the optional is not pointer-like and payload has bits.
......@@ -9369,7 +9377,10 @@ fn intrinsicsAllowed(scalar_ty: Type, target: std.Target) bool {
93699377/// We can do this because for all types, Zig ABI alignment >= LLVM ABI
93709378/// alignment.
93719379const struct_layout_version = 2;
9372const optional_layout_version = 2;
9380
9381// TODO: Restore the non_null field to i1 once
9382// https://github.com/llvm/llvm-project/issues/56585/ is fixed
9383const optional_layout_version = 3;
93739384
93749385/// We use the least significant bit of the pointer address to tell us
93759386/// whether the type is fully resolved. Types that are only fwd declared