Skip to content

Commit b93a388

Browse files
topolarityandrewrk
authored andcommitted
stage2: Change optional non-null field to i8
This is a workaround for llvm/llvm-project#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 parent 0e26c61 commit b93a388

File tree

1 file changed

+45
-34
lines changed

1 file changed

+45
-34
lines changed

src/codegen/llvm.zig

Lines changed: 45 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -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.
93719379
const struct_layout_version = 2;
9372-
const 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
9383+
const 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

0 commit comments

Comments
 (0)