Skip to content

Commit

Permalink
Revert "Merge pull request #13914 from Vexu/variadic"
Browse files Browse the repository at this point in the history 
This reverts commit aca9c74, reversing
changes made to d93edad.

These changes were not tested with LLVM with assertions enabled. They
caused a regression in master branch, triggering LLVM assertions when
running behavior tests.

Reopens #515
  • Loading branch information
@andrewrk
andrewrk committed Dec 28, 2022
1 parent d770a47 commit e08efc2
Show file tree
Hide file tree
Showing 25 changed files with 25 additions and 660 deletions.
59 changes: 6 additions & 53 deletions doc/langref.html.in
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8137,35 +8137,6 @@ test "main" {
{#see_also|Import from C Header File|@cImport|@cDefine|@cInclude#}
{#header_close#}

{#header_open|@cVaArg#}
<pre>{#syntax#}@cVaArg(operand: *std.builtin.VaList, comptime T: type) T{#endsyntax#}</pre>
<p>
Implements the C macro {#syntax#}va_arg{#endsyntax#}.
</p>
{#see_also|@cVaCopy|@cVaEnd|@cVaStart#}
{#header_close#}
{#header_open|@cVaCopy#}
<pre>{#syntax#}@cVaCopy(src: *std.builtin.VaList) std.builtin.VaList{#endsyntax#}</pre>
<p>
Implements the C macro {#syntax#}va_copy{#endsyntax#}.
</p>
{#see_also|@cVaArg|@cVaEnd|@cVaStart#}
{#header_close#}
{#header_open|@cVaEnd#}
<pre>{#syntax#}@cVaEnd(src: *std.builtin.VaList) void{#endsyntax#}</pre>
<p>
Implements the C macro {#syntax#}va_end{#endsyntax#}.
</p>
{#see_also|@cVaArg|@cVaCopy|@cVaStart#}
{#header_close#}
{#header_open|@cVaStart#}
<pre>{#syntax#}@cVaStart() std.builtin.VaList{#endsyntax#}</pre>
<p>
Implements the C macro {#syntax#}va_start{#endsyntax#}. Only valid inside a variadic function.
</p>
{#see_also|@cVaArg|@cVaCopy|@cVaEnd#}
{#header_close#}

{#header_open|@divExact#}
<pre>{#syntax#}@divExact(numerator: T, denominator: T) T{#endsyntax#}</pre>
<p>
Expand Down Expand Up @@ -10874,32 +10845,14 @@ test "variadic function" {
}
{#code_end#}
<p>
Variadic functions can be implemented using {#link|@cVaStart#}, {#link|@cVaEnd#}, {#link|@cVaArg#} and {#link|@cVaCopy#}
Non extern variadic functions are currently not implemented, but there
is an accepted proposal. See <a href="https://github.com/ziglang/zig/issues/515">#515</a>.
</p>
{#code_begin|test|defining_variadic_function#}
const std = @import("std");
const testing = std.testing;
const builtin = @import("builtin");
{#code_begin|obj_err|non-extern function is variadic#}
export fn printf(format: [*:0]const u8, ...) c_int {
_ = format;

fn add(count: c_int, ...) callconv(.C) c_int {
var ap = @cVaStart();
defer @cVaEnd(&ap);
var i: usize = 0;
var sum: c_int = 0;
while (i < count) : (i += 1) {
sum += @cVaArg(&ap, c_int);
}
return sum;
}

test "defining a variadic function" {
// Variadic functions are currently disabled on some targets due to miscompilations.
if (builtin.cpu.arch == .aarch64 and builtin.os.tag != .windows and builtin.os.tag != .macos) return error.SkipZigTest;
if (builtin.cpu.arch == .x86_64 and builtin.os.tag == .windows) return error.SkipZigTest;

try std.testing.expectEqual(@as(c_int, 0), add(0));
try std.testing.expectEqual(@as(c_int, 1), add(1, @as(c_int, 1)));
try std.testing.expectEqual(@as(c_int, 3), add(2, @as(c_int, 1), @as(c_int, 2)));
return 0;
}
{#code_end#}
{#header_close#}
Expand Down
81 changes: 0 additions & 81 deletions lib/std/builtin.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -620,87 +620,6 @@ pub const CallModifier = enum {
compile_time,
};

/// This data structure is used by the Zig language code generation and
/// therefore must be kept in sync with the compiler implementation.
pub const VaListAarch64 = extern struct {
__stack: *anyopaque,
__gr_top: *anyopaque,
__vr_top: *anyopaque,
__gr_offs: c_int,
__vr_offs: c_int,
};

/// This data structure is used by the Zig language code generation and
/// therefore must be kept in sync with the compiler implementation.
pub const VaListHexagon = extern struct {
__gpr: c_long,
__fpr: c_long,
__overflow_arg_area: *anyopaque,
__reg_save_area: *anyopaque,
};

/// This data structure is used by the Zig language code generation and
/// therefore must be kept in sync with the compiler implementation.
pub const VaListPowerPc = extern struct {
gpr: u8,
fpr: u8,
reserved: c_ushort,
overflow_arg_area: *anyopaque,
reg_save_area: *anyopaque,
};

/// This data structure is used by the Zig language code generation and
/// therefore must be kept in sync with the compiler implementation.
pub const VaListS390x = extern struct {
__current_saved_reg_area_pointer: *anyopaque,
__saved_reg_area_end_pointer: *anyopaque,
__overflow_area_pointer: *anyopaque,
};

/// This data structure is used by the Zig language code generation and
/// therefore must be kept in sync with the compiler implementation.
pub const VaListX86_64 = extern struct {
gp_offset: c_uint,
fp_offset: c_uint,
overflow_arg_area: *anyopaque,
reg_save_area: *anyopaque,
};

/// This data structure is used by the Zig language code generation and
/// therefore must be kept in sync with the compiler implementation.
pub const VaList = switch (builtin.cpu.arch) {
.aarch64 => switch (builtin.os.tag) {
.windows => *u8,
.ios, .macos, .tvos, .watchos => *u8,
else => @compileError("disabled due to miscompilations"), // VaListAarch64,
},
.arm => switch (builtin.os.tag) {
.ios, .macos, .tvos, .watchos => *u8,
else => *anyopaque,
},
.amdgcn => *u8,
.avr => *anyopaque,
.bpfel, .bpfeb => *anyopaque,
.hexagon => if (builtin.target.isMusl()) VaListHexagon else *u8,
.mips, .mipsel, .mips64, .mips64el => *anyopaque,
.riscv32, .riscv64 => *anyopaque,
.powerpc, .powerpcle => switch (builtin.os.tag) {
.ios, .macos, .tvos, .watchos, .aix => *u8,
else => VaListPowerPc,
},
.powerpc64, .powerpc64le => *u8,
.sparc, .sparcel, .sparc64 => *anyopaque,
.spirv32, .spirv64 => *anyopaque,
.s390x => VaListS390x,
.wasm32, .wasm64 => *anyopaque,
.x86 => *u8,
.x86_64 => switch (builtin.os.tag) {
.windows => @compileError("disabled due to miscompilations"), // *u8,
else => VaListX86_64,
},
else => @compileError("VaList not supported for this target yet"),
};

/// This data structure is used by the Zig language code generation and
/// therefore must be kept in sync with the compiler implementation.
pub const PrefetchOptions = struct {
Expand Down
17 changes: 0 additions & 17 deletions src/Air.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -741,19 +741,6 @@ pub const Inst = struct {
/// Uses the `vector_store_elem` field.
vector_store_elem,

/// Implements @cVaArg builtin.
/// Uses the `ty_op` field.
c_va_arg,
/// Implements @cVaCopy builtin.
/// Uses the `ty_op` field.
c_va_copy,
/// Implements @cVaEnd builtin.
/// Uses the `un_op` field.
c_va_end,
/// Implements @cVaStart builtin.
/// Uses the `ty` field.
c_va_start,

pub fn fromCmpOp(op: std.math.CompareOperator, optimized: bool) Tag {
switch (op) {
.lt => return if (optimized) .cmp_lt_optimized else .cmp_lt,
Expand Down Expand Up @@ -1105,7 +1092,6 @@ pub fn typeOfIndex(air: Air, inst: Air.Inst.Index) Type {
.ret_ptr,
.arg,
.err_return_trace,
.c_va_start,
=> return datas[inst].ty,

.assembly,
Expand Down Expand Up @@ -1170,8 +1156,6 @@ pub fn typeOfIndex(air: Air, inst: Air.Inst.Index) Type {
.byte_swap,
.bit_reverse,
.addrspace_cast,
.c_va_arg,
.c_va_copy,
=> return air.getRefType(datas[inst].ty_op.ty),

.loop,
Expand Down Expand Up @@ -1203,7 +1187,6 @@ pub fn typeOfIndex(air: Air, inst: Air.Inst.Index) Type {
.prefetch,
.set_err_return_trace,
.vector_store_elem,
.c_va_end,
=> return Type.void,

.ptrtoint,
Expand Down
53 changes: 6 additions & 47 deletions src/AstGen.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ string_table: std.HashMapUnmanaged(u32, void, StringIndexContext, std.hash_map.d
compile_errors: ArrayListUnmanaged(Zir.Inst.CompileErrors.Item) = .{},
/// The topmost block of the current function.
fn_block: ?*GenZir = null,
fn_var_args: bool = false,
/// Maps string table indexes to the first `@import` ZIR instruction
/// that uses this string as the operand.
imports: std.AutoArrayHashMapUnmanaged(u32, Ast.TokenIndex) = .{},
Expand Down Expand Up @@ -3891,17 +3890,17 @@ fn fnDecl(
.noalias_bits = noalias_bits,
});
} else func: {
if (is_var_args) {
return astgen.failTok(fn_proto.ast.fn_token, "non-extern function is variadic", .{});
}

// as a scope, fn_gz encloses ret_gz, but for instruction list, fn_gz stacks on ret_gz
fn_gz.instructions_top = ret_gz.instructions.items.len;

const prev_fn_block = astgen.fn_block;
astgen.fn_block = &fn_gz;
defer astgen.fn_block = prev_fn_block;

const prev_var_args = astgen.fn_var_args;
astgen.fn_var_args = is_var_args;
defer astgen.fn_var_args = prev_var_args;

astgen.advanceSourceCursorToNode(body_node);
const lbrace_line = astgen.source_line - decl_gz.decl_line;
const lbrace_column = astgen.source_column;
Expand Down Expand Up @@ -6070,15 +6069,15 @@ fn whileExpr(
const tag: Zir.Inst.Tag = if (payload_is_ref) .is_non_err_ptr else .is_non_err;
break :c .{
.inst = err_union,
.bool_bit = try cond_scope.addUnNode(tag, err_union, while_full.ast.cond_expr),
.bool_bit = try cond_scope.addUnNode(tag, err_union, while_full.ast.then_expr),
};
} else if (while_full.payload_token) |_| {
const cond_ri: ResultInfo = .{ .rl = if (payload_is_ref) .ref else .none };
const optional = try expr(&cond_scope, &cond_scope.base, cond_ri, while_full.ast.cond_expr);
const tag: Zir.Inst.Tag = if (payload_is_ref) .is_non_null_ptr else .is_non_null;
break :c .{
.inst = optional,
.bool_bit = try cond_scope.addUnNode(tag, optional, while_full.ast.cond_expr),
.bool_bit = try cond_scope.addUnNode(tag, optional, while_full.ast.then_expr),
};
} else {
const cond = try expr(&cond_scope, &cond_scope.base, bool_ri, while_full.ast.cond_expr);
Expand Down Expand Up @@ -8369,46 +8368,6 @@ fn builtinCall(
});
return rvalue(gz, ri, result, node);
},
.c_va_arg => {
if (astgen.fn_block == null) {
return astgen.failNode(node, "'@cVaArg' outside function scope", .{});
}
const result = try gz.addExtendedPayload(.c_va_arg, Zir.Inst.BinNode{
.node = gz.nodeIndexToRelative(node),
.lhs = try expr(gz, scope, .{ .rl = .none }, params[0]),
.rhs = try typeExpr(gz, scope, params[1]),
});
return rvalue(gz, ri, result, node);
},
.c_va_copy => {
if (astgen.fn_block == null) {
return astgen.failNode(node, "'@cVaCopy' outside function scope", .{});
}
const result = try gz.addExtendedPayload(.c_va_copy, Zir.Inst.UnNode{
.node = gz.nodeIndexToRelative(node),
.operand = try expr(gz, scope, .{ .rl = .none }, params[0]),
});
return rvalue(gz, ri, result, node);
},
.c_va_end => {
if (astgen.fn_block == null) {
return astgen.failNode(node, "'@cVaEnd' outside function scope", .{});
}
const result = try gz.addExtendedPayload(.c_va_end, Zir.Inst.UnNode{
.node = gz.nodeIndexToRelative(node),
.operand = try expr(gz, scope, .{ .rl = .none }, params[0]),
});
return rvalue(gz, ri, result, node);
},
.c_va_start => {
if (astgen.fn_block == null) {
return astgen.failNode(node, "'@cVaStart' outside function scope", .{});
}
if (!astgen.fn_var_args) {
return astgen.failNode(node, "'@cVaStart' in a non-variadic function", .{});
}
return rvalue(gz, ri, try gz.addNodeExtended(.c_va_start, node), node);
},
}
}

Expand Down
28 changes: 0 additions & 28 deletions src/BuiltinFn.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,6 @@ pub const Tag = enum {
compile_log,
ctz,
c_undef,
c_va_arg,
c_va_copy,
c_va_end,
c_va_start,
div_exact,
div_floor,
div_trunc,
Expand Down Expand Up @@ -358,30 +354,6 @@ pub const list = list: {
.param_count = 1,
},
},
.{
"@cVaArg", .{
.tag = .c_va_arg,
.param_count = 2,
},
},
.{
"@cVaCopy", .{
.tag = .c_va_copy,
.param_count = 1,
},
},
.{
"@cVaEnd", .{
.tag = .c_va_end,
.param_count = 1,
},
},
.{
"@cVaStart", .{
.tag = .c_va_start,
.param_count = 0,
},
},
.{
"@divExact",
.{
Expand Down
8 changes: 0 additions & 8 deletions src/Liveness.zig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@ pub fn categorizeOperand(
.wasm_memory_size,
.err_return_trace,
.save_err_return_trace_index,
.c_va_start,
=> return .none,

.fence => return .write,
Expand Down Expand Up @@ -280,8 +279,6 @@ pub fn categorizeOperand(
.splat,
.error_set_has_value,
.addrspace_cast,
.c_va_arg,
.c_va_copy,
=> {
const o = air_datas[inst].ty_op;
if (o.operand == operand_ref) return matchOperandSmallIndex(l, inst, 0, .none);
Expand Down Expand Up @@ -325,7 +322,6 @@ pub fn categorizeOperand(
.trunc_float,
.neg,
.cmp_lt_errors_len,
.c_va_end,
=> {
const o = air_datas[inst].un_op;
if (o == operand_ref) return matchOperandSmallIndex(l, inst, 0, .none);
Expand Down Expand Up @@ -861,7 +857,6 @@ fn analyzeInst(
.wasm_memory_size,
.err_return_trace,
.save_err_return_trace_index,
.c_va_start,
=> return trackOperands(a, new_set, inst, main_tomb, .{ .none, .none, .none }),

.not,
Expand Down Expand Up @@ -903,8 +898,6 @@ fn analyzeInst(
.splat,
.error_set_has_value,
.addrspace_cast,
.c_va_arg,
.c_va_copy,
=> {
const o = inst_datas[inst].ty_op;
return trackOperands(a, new_set, inst, main_tomb, .{ o.operand, .none, .none });
Expand Down Expand Up @@ -943,7 +936,6 @@ fn analyzeInst(
.neg_optimized,
.cmp_lt_errors_len,
.set_err_return_trace,
.c_va_end,
=> {
const operand = inst_datas[inst].un_op;
return trackOperands(a, new_set, inst, main_tomb, .{ operand, .none, .none });
Expand Down
Loading

0 comments on commit e08efc2

Please sign in to comment.