Commit d5c1d24964

Andrew Kelley <andrew@ziglang.org>
2021-09-16 04:55:57
stage2: fix "cmpxchg with ptr" test case
* Sema: fix atomic operand checking to allow pointers. * LLVM backend: implement pointer-like optional constants. * LLVM backend: fix `is_non_null` and `optional_payload` instructions to support pointer-like optionals. * Type: introduce `isPtrAtRuntime` method. * Type: fix `isPtrLikeOptional` to get the correct answer for allowzero pointers and slices.
1 parent b67d181
Changed files (5)
src/codegen/llvm.zig
@@ -602,18 +602,18 @@ pub const DeclGen = struct {
                 return elem_type.arrayType(@intCast(c_uint, total_len));
             },
             .Optional => {
-                if (!t.isPtrLikeOptional()) {
-                    var buf: Type.Payload.ElemType = undefined;
-                    const child_type = t.optionalChild(&buf);
+                var buf: Type.Payload.ElemType = undefined;
+                const child_type = t.optionalChild(&buf);
+                const payload_llvm_ty = try self.llvmType(child_type);
 
-                    const optional_types: [2]*const llvm.Type = .{
-                        try self.llvmType(child_type),
-                        self.context.intType(1),
-                    };
-                    return self.context.structType(&optional_types, 2, .False);
-                } else {
-                    return self.todo("implement optional pointers as actual pointers", .{});
+                if (t.isPtrLikeOptional()) {
+                    return payload_llvm_ty;
                 }
+
+                const fields: [2]*const llvm.Type = .{
+                    payload_llvm_ty, self.context.intType(1),
+                };
+                return self.context.structType(&fields, fields.len, .False);
             },
             .ErrorUnion => {
                 const error_type = t.errorUnionSet();
@@ -1573,6 +1573,13 @@ pub const FuncGen = struct {
         const operand = try self.resolveInst(un_op);
 
         if (operand_is_ptr) {
+            const operand_ty = self.air.typeOf(un_op).elemType();
+            if (operand_ty.isPtrLikeOptional()) {
+                const operand_llvm_ty = try self.dg.llvmType(operand_ty);
+                const loaded = self.builder.buildLoad(operand, "");
+                return self.builder.buildICmp(.NE, loaded, operand_llvm_ty.constNull(), "");
+            }
+
             const index_type = self.context.intType(32);
 
             var indices: [2]*const llvm.Value = .{
@@ -1581,9 +1588,15 @@ pub const FuncGen = struct {
             };
 
             return self.builder.buildLoad(self.builder.buildInBoundsGEP(operand, &indices, indices.len, ""), "");
-        } else {
-            return self.builder.buildExtractValue(operand, 1, "");
         }
+
+        const operand_ty = self.air.typeOf(un_op);
+        if (operand_ty.isPtrLikeOptional()) {
+            const operand_llvm_ty = try self.dg.llvmType(operand_ty);
+            return self.builder.buildICmp(.NE, operand, operand_llvm_ty.constNull(), "");
+        }
+
+        return self.builder.buildExtractValue(operand, 1, "");
     }
 
     fn airIsNull(self: *FuncGen, inst: Air.Inst.Index, operand_is_ptr: bool) !?*const llvm.Value {
@@ -1636,17 +1649,24 @@ pub const FuncGen = struct {
         const operand = try self.resolveInst(ty_op.operand);
 
         if (operand_is_ptr) {
-            const index_type = self.context.intType(32);
+            const operand_ty = self.air.typeOf(ty_op.operand).elemType();
+            if (operand_ty.isPtrLikeOptional()) {
+                return self.builder.buildLoad(operand, "");
+            }
 
+            const index_type = self.context.intType(32);
             var indices: [2]*const llvm.Value = .{
-                index_type.constNull(),
-                index_type.constNull(),
+                index_type.constNull(), index_type.constNull(),
             };
-
             return self.builder.buildInBoundsGEP(operand, &indices, 2, "");
-        } else {
-            return self.builder.buildExtractValue(operand, 0, "");
         }
+
+        const operand_ty = self.air.typeOf(ty_op.operand);
+        if (operand_ty.isPtrLikeOptional()) {
+            return operand;
+        }
+
+        return self.builder.buildExtractValue(operand, 0, "");
     }
 
     fn airErrUnionPayload(
@@ -2050,8 +2070,6 @@ pub const FuncGen = struct {
         );
 
         const optional_ty = self.air.typeOfIndex(inst);
-        var buffer: Type.Payload.ElemType = undefined;
-        const child_ty = optional_ty.optionalChild(&buffer);
 
         var payload = self.builder.buildExtractValue(result, 0, "");
         if (opt_abi_ty != null) {
@@ -2060,8 +2078,7 @@ pub const FuncGen = struct {
         const success_bit = self.builder.buildExtractValue(result, 1, "");
 
         if (optional_ty.isPtrLikeOptional()) {
-            const child_llvm_ty = try self.dg.llvmType(child_ty);
-            return self.builder.buildSelect(success_bit, child_llvm_ty.constNull(), payload, "");
+            return self.builder.buildSelect(success_bit, payload.typeOf().constNull(), payload, "");
         }
 
         const optional_llvm_ty = try self.dg.llvmType(optional_ty);
src/Sema.zig
@@ -7518,12 +7518,16 @@ fn checkAtomicOperandType(
             return;
         },
         .Bool => return, // Will be treated as `u8`.
-        else => return sema.mod.fail(
-            &block.base,
-            ty_src,
-            "expected bool, integer, float, enum, or pointer type; found {}",
-            .{ty},
-        ),
+        else => {
+            if (ty.isPtrAtRuntime()) return;
+
+            return sema.mod.fail(
+                &block.base,
+                ty_src,
+                "expected bool, integer, float, enum, or pointer type; found {}",
+                .{ty},
+            );
+        },
     };
     const bit_count = int_ty.intInfo(target).bits;
     if (bit_count > max_atomic_bits) {
src/type.zig
@@ -2191,6 +2191,44 @@ pub const Type = extern union {
         };
     }
 
+    pub fn isPtrAtRuntime(self: Type) bool {
+        switch (self.tag()) {
+            .c_const_pointer,
+            .c_mut_pointer,
+            .many_const_pointer,
+            .many_mut_pointer,
+            .manyptr_const_u8,
+            .manyptr_u8,
+            .optional_single_const_pointer,
+            .optional_single_mut_pointer,
+            .single_const_pointer,
+            .single_const_pointer_to_comptime_int,
+            .single_mut_pointer,
+            => return true,
+
+            .pointer => switch (self.castTag(.pointer).?.data.size) {
+                .Slice => return false,
+                .One, .Many, .C => return true,
+            },
+
+            .optional => {
+                var buf: Payload.ElemType = undefined;
+                const child_type = self.optionalChild(&buf);
+                // optionals of zero sized pointers behave like bools
+                if (!child_type.hasCodeGenBits()) return false;
+                if (child_type.zigTypeTag() != .Pointer) return false;
+
+                const info = child_type.ptrInfo().data;
+                switch (info.size) {
+                    .Slice, .C => return false,
+                    .Many, .One => return !info.@"allowzero",
+                }
+            },
+
+            else => return false,
+        }
+    }
+
     /// Asserts that the type is an optional
     pub fn isPtrLikeOptional(self: Type) bool {
         switch (self.tag()) {
@@ -2203,8 +2241,13 @@ pub const Type = extern union {
                 const child_type = self.optionalChild(&buf);
                 // optionals of zero sized pointers behave like bools
                 if (!child_type.hasCodeGenBits()) return false;
+                if (child_type.zigTypeTag() != .Pointer) return false;
 
-                return child_type.zigTypeTag() == .Pointer and !child_type.isCPtr();
+                const info = child_type.ptrInfo().data;
+                switch (info.size) {
+                    .Slice, .C => return false,
+                    .Many, .One => return !info.@"allowzero",
+                }
             },
             else => unreachable,
         }
test/behavior/atomics.zig
@@ -53,3 +53,23 @@ fn testAtomicLoad(ptr: *u8) !void {
     const x = @atomicLoad(u8, ptr, .SeqCst);
     try expect(x == 42);
 }
+
+test "cmpxchg with ptr" {
+    var data1: i32 = 1234;
+    var data2: i32 = 5678;
+    var data3: i32 = 9101;
+    var x: *i32 = &data1;
+    if (@cmpxchgWeak(*i32, &x, &data2, &data3, .SeqCst, .SeqCst)) |x1| {
+        try expect(x1 == &data1);
+    } else {
+        @panic("cmpxchg should have failed");
+    }
+
+    while (@cmpxchgWeak(*i32, &x, &data1, &data3, .SeqCst, .SeqCst)) |x1| {
+        try expect(x1 == &data1);
+    }
+    try expect(x == &data3);
+
+    try expect(@cmpxchgStrong(*i32, &x, &data3, &data2, .SeqCst, .SeqCst) == null);
+    try expect(x == &data2);
+}
test/behavior/atomics_stage1.zig
@@ -3,26 +3,6 @@ const expect = std.testing.expect;
 const expectEqual = std.testing.expectEqual;
 const builtin = @import("builtin");
 
-test "cmpxchg with ptr" {
-    var data1: i32 = 1234;
-    var data2: i32 = 5678;
-    var data3: i32 = 9101;
-    var x: *i32 = &data1;
-    if (@cmpxchgWeak(*i32, &x, &data2, &data3, .SeqCst, .SeqCst)) |x1| {
-        try expect(x1 == &data1);
-    } else {
-        @panic("cmpxchg should have failed");
-    }
-
-    while (@cmpxchgWeak(*i32, &x, &data1, &data3, .SeqCst, .SeqCst)) |x1| {
-        try expect(x1 == &data1);
-    }
-    try expect(x == &data3);
-
-    try expect(@cmpxchgStrong(*i32, &x, &data3, &data2, .SeqCst, .SeqCst) == null);
-    try expect(x == &data2);
-}
-
 test "128-bit cmpxchg" {
     try test_u128_cmpxchg();
     comptime try test_u128_cmpxchg();