From 5d5282b5f18b33489187a9a9e23a1aa2d301b4e8 Mon Sep 17 00:00:00 2001 From: Daniele Cocca Date: Sat, 19 Mar 2022 12:00:07 +0000 Subject: [PATCH 1/4] AstGen: support local var references for outputs --- src/AstGen.zig | 50 ++++++++++++++++---------------------------------- 1 file changed, 16 insertions(+), 34 deletions(-) diff --git a/src/AstGen.zig b/src/AstGen.zig index ed6c9f86ce..296eb80e7c 100644 --- a/src/AstGen.zig +++ b/src/AstGen.zig @@ -6423,7 +6423,6 @@ fn identifier( const astgen = gz.astgen; const tree = astgen.tree; - const gpa = astgen.gpa; const main_tokens = tree.nodes.items(.main_token); const ident_token = main_tokens[ident]; @@ -6467,6 +6466,19 @@ fn identifier( } // Local variables, including function parameters. + return localVarRef(gz, scope, rl, ident, ident_token); +} + +fn localVarRef( + gz: *GenZir, + scope: *Scope, + rl: ResultLoc, + ident: Ast.Node.Index, + ident_token: Ast.Node.Index, +) InnerError!Zir.Inst.Ref { + const astgen = gz.astgen; + const gpa = astgen.gpa; + const name_str_index = try astgen.identAsString(ident_token); var s = scope; var found_already: ?Ast.Node.Index = null; // we have found a decl with the same name already @@ -6808,43 +6820,13 @@ fn asmExpr( }; } else { const ident_token = symbolic_name + 4; - const str_index = try astgen.identAsString(ident_token); - // TODO this needs extra code for local variables. Have a look at #215 and related - // issues and decide how to handle outputs. Do we want this to be identifiers? + // TODO have a look at #215 and related issues and decide how to + // handle outputs. Do we want this to be identifiers? // Or maybe we want to force this to be expressions with a pointer type. - // Until that is figured out this is only hooked up for referencing Decls. - // TODO we have put this as an identifier lookup just so that we don't get - // unused vars for outputs. We need to check if this is correct in the future ^^ - // so we just put in this simple lookup. This is a workaround. - { - var s = scope; - while (true) switch (s.tag) { - .local_val => { - const local_val = s.cast(Scope.LocalVal).?; - if (local_val.name == str_index) { - local_val.used = true; - break; - } - s = local_val.parent; - }, - .local_ptr => { - const local_ptr = s.cast(Scope.LocalPtr).?; - if (local_ptr.name == str_index) { - local_ptr.used = true; - break; - } - s = local_ptr.parent; - }, - .gen_zir => s = s.cast(GenZir).?.parent, - .defer_normal, .defer_error => s = s.cast(Scope.Defer).?.parent, - .namespace, .top => break, - }; - } - const operand = try gz.addStrTok(.decl_ref, str_index, ident_token); outputs[i] = .{ .name = name, .constraint = constraint, - .operand = operand, + .operand = try localVarRef(gz, scope, rl, node, ident_token), }; } } From 633fe41a2c2310d8cfab53ee9d87bb50ac4efc41 Mon Sep 17 00:00:00 2001 From: Daniele Cocca Date: Sun, 20 Mar 2022 20:50:59 +0000 Subject: [PATCH 2/4] Sema: allow comptime blocks for global assembly An assembly expression in a comptime block is legal Zig in the case of global assembly [^1]. Instead of unconditionally asserting that the expression lives in a runtime block, here we assert that if the expression lives in a comptime block it must be outside of function scope. [^1]: https://ziglang.org/documentation/0.9.1/#Global-Assembly --- src/Sema.zig | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Sema.zig b/src/Sema.zig index 9fc54f805b..f13f97bbbb 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -10253,6 +10253,11 @@ fn zirAsm( const inputs_len = @truncate(u5, extended.small >> 5); const clobbers_len = @truncate(u5, extended.small >> 10); const is_volatile = @truncate(u1, extended.small >> 15) != 0; + const is_global_assembly = sema.func == null; + + if (block.is_comptime and !is_global_assembly) { + try sema.requireRuntimeBlock(block, src); + } if (extra.data.asm_source == 0) { // This can move to become an AstGen error after inline assembly improvements land @@ -10317,7 +10322,6 @@ fn zirAsm( needed_capacity += (asm_source.len + 3) / 4; const gpa = sema.gpa; - try sema.requireRuntimeBlock(block, src); try sema.air_extra.ensureUnusedCapacity(gpa, needed_capacity); const asm_air = try block.addInst(.{ .tag = .assembly, From ebafdb958c1aa6b41c28fc7d45f44e38a69a3bd5 Mon Sep 17 00:00:00 2001 From: Daniele Cocca Date: Sun, 20 Mar 2022 20:57:56 +0000 Subject: [PATCH 3/4] AstGen: don't coerce inputs to usize in asmExpr Instead, use ResultLoc.none to allow for the expression type to be inferred [^1]. This effectively moves the type coercion to Sema, in order to turn comptime values into usable values for the backends to consume. Right now the coercion is applies as comptime_int -> usize and comptime_float -> f64, as an arbitrary choice. [^1]: https://github.com/ziglang/zig/blob/9f25c8140cb859fcea7023362afcb29b1e4df41f/src/AstGen.zig#L207-L208 --- src/AstGen.zig | 2 +- src/Sema.zig | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/AstGen.zig b/src/AstGen.zig index 296eb80e7c..b66f7b868b 100644 --- a/src/AstGen.zig +++ b/src/AstGen.zig @@ -6842,7 +6842,7 @@ fn asmExpr( const name = try astgen.identAsString(symbolic_name); const constraint_token = symbolic_name + 2; const constraint = (try astgen.strLitAsString(constraint_token)).index; - const operand = try expr(gz, scope, .{ .ty = .usize_type }, node_datas[input_node].lhs); + const operand = try expr(gz, scope, .none, node_datas[input_node].lhs); inputs[i] = .{ .name = name, .constraint = constraint, diff --git a/src/Sema.zig b/src/Sema.zig index f13f97bbbb..112939c995 100644 --- a/src/Sema.zig +++ b/src/Sema.zig @@ -10304,7 +10304,14 @@ fn zirAsm( const name = sema.code.nullTerminatedString(input.data.name); _ = name; // TODO: use the name - arg.* = sema.resolveInst(input.data.operand); + const uncasted_arg = sema.resolveInst(input.data.operand); + const uncasted_arg_ty = sema.typeOf(uncasted_arg); + switch (uncasted_arg_ty.zigTypeTag()) { + .ComptimeInt => arg.* = try sema.coerce(block, Type.initTag(.usize), uncasted_arg, src), + .ComptimeFloat => arg.* = try sema.coerce(block, Type.initTag(.f64), uncasted_arg, src), + else => arg.* = uncasted_arg, + } + const constraint = sema.code.nullTerminatedString(input.data.constraint); needed_capacity += constraint.len / 4 + 1; inputs[arg_i] = constraint; From 907dc1e13f657f349dbdecf739b2f1a13ad7011a Mon Sep 17 00:00:00 2001 From: Daniele Cocca Date: Sun, 20 Mar 2022 21:04:28 +0000 Subject: [PATCH 4/4] CBE: improve support for asm inputs This is not complete support for asm expressions, but allows a few more test cases from test/behavior/asm.zig to pass. Since the non-register inputs are named `input_${n}` they can cause name collisions: I'm wrapping the asm expressions in their own block to prevent that. Contextually, this change also makes test/behavior/asm.zig run for stage2, but skips individual tests for most backends (I only verified the C and LLVM backends successfully run one new test case) and the entire test file for aarch64, where it's running into preexisting shortcomings. --- src/codegen/c.zig | 18 +++++++++++++----- test/behavior.zig | 5 +---- test/behavior/asm.zig | 38 +++++++++++++++++++++++++++++++++++++- 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/src/codegen/c.zig b/src/codegen/c.zig index 19556fe8c4..4085305941 100644 --- a/src/codegen/c.zig +++ b/src/codegen/c.zig @@ -3014,9 +3014,10 @@ fn airAsm(f: *Function, inst: Air.Inst.Index) !CValue { } else null; const writer = f.object.writer(); - const inputs_extra_begin = extra_i; + try writer.writeAll("{\n"); - for (inputs) |input| { + const inputs_extra_begin = extra_i; + for (inputs) |input, i| { const constraint = std.mem.sliceTo(std.mem.sliceAsBytes(f.air.extra[extra_i..]), 0); // This equation accounts for the fact that even if we have exactly 4 bytes // for the string, we still use the next u32 for the null terminator. @@ -3032,7 +3033,11 @@ fn airAsm(f: *Function, inst: Air.Inst.Index) !CValue { try f.writeCValue(writer, arg_c_value); try writer.writeAll(";\n"); } else { - return f.fail("TODO non-explicit inline asm regs", .{}); + try writer.writeAll("register "); + try f.renderType(writer, f.air.typeOf(input)); + try writer.print(" input_{d} = ", .{i}); + try f.writeCValue(writer, try f.resolveInst(input)); + try writer.writeAll(";\n"); } } @@ -3074,12 +3079,15 @@ fn airAsm(f: *Function, inst: Air.Inst.Index) !CValue { } try writer.print("\"r\"({s}_constant)", .{reg}); } else { - // This is blocked by the earlier test - unreachable; + if (index > 0) { + try writer.writeAll(", "); + } + try writer.print("\"r\"(input_{d})", .{index}); } } } try writer.writeAll(");\n"); + try writer.writeAll("}\n"); if (f.liveness.isUnused(inst)) return CValue.none; diff --git a/test/behavior.zig b/test/behavior.zig index 60b8d2bae7..de61efa482 100644 --- a/test/behavior.zig +++ b/test/behavior.zig @@ -165,10 +165,7 @@ test { } if (builtin.os.tag != .wasi) { - if (builtin.zig_backend == .stage1) { - // TODO get these tests passing with stage2 - _ = @import("behavior/asm.zig"); - } + _ = @import("behavior/asm.zig"); } if (builtin.zig_backend != .stage2_arm and diff --git a/test/behavior/asm.zig b/test/behavior/asm.zig index f856f0128e..dab6f12127 100644 --- a/test/behavior/asm.zig +++ b/test/behavior/asm.zig @@ -5,7 +5,10 @@ const expect = std.testing.expect; const is_x86_64_linux = builtin.cpu.arch == .x86_64 and builtin.os.tag == .linux; comptime { - if (is_x86_64_linux) { + if (builtin.zig_backend != .stage2_arm and + builtin.zig_backend != .stage2_aarch64 and + is_x86_64_linux) + { asm ( \\.globl this_is_my_alias; \\.type this_is_my_alias, @function; @@ -15,12 +18,26 @@ comptime { } test "module level assembly" { + if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_llvm) return error.SkipZigTest; // TODO + if (is_x86_64_linux) { try expect(this_is_my_alias() == 1234); } } test "output constraint modifiers" { + if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_llvm) return error.SkipZigTest; // TODO + // This is only testing compilation. var a: u32 = 3; asm volatile ("" @@ -36,6 +53,13 @@ test "output constraint modifiers" { } test "alternative constraints" { + if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_llvm) return error.SkipZigTest; // TODO + // Make sure we allow commas as a separator for alternative constraints. var a: u32 = 3; asm volatile ("" @@ -46,6 +70,11 @@ test "alternative constraints" { } test "sized integer/float in asm input" { + if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + asm volatile ("" : : [_] "m" (@as(usize, 3)), @@ -89,6 +118,13 @@ test "sized integer/float in asm input" { } test "struct/array/union types as input values" { + if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_x86_64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_aarch64) return error.SkipZigTest; // TODO + if (builtin.zig_backend == .stage2_llvm) return error.SkipZigTest; // TODO + asm volatile ("" : : [_] "m" (@as([1]u32, undefined)),