From 7efc2a06264170632e56256a5fad97e945768056 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 28 Sep 2021 20:33:50 -0700 Subject: [PATCH] AstGen: improved logic for nodeMayNeedMemoryLocation * `@as` and `@bitCast` no longer unconditionally return `true` from this function; they forward the question to their sub-expression. * fix `@splat` incorrectly being marked as needing a memory location (this function returns a SIMD vector; it definitely does not want a memory location). Makes AstGen generate slightly nicer ZIR, which in turn generates slightly nicer AIR, generating slightly nicer machine code in debug builds. It also means I can procrastinate implementing the bitcast_result_ptr ZIR instruction semantic analysis :^) --- src/AstGen.zig | 26 ++++++++++++++++++++------ src/BuiltinFn.zig | 26 +++++++++++++++++--------- 2 files changed, 37 insertions(+), 15 deletions(-) diff --git a/src/AstGen.zig b/src/AstGen.zig index 847860630a..24a73539b3 100644 --- a/src/AstGen.zig +++ b/src/AstGen.zig @@ -8271,17 +8271,31 @@ fn nodeMayNeedMemoryLocation(tree: *const Ast, start_node: Ast.Node.Index) bool } }, - .builtin_call, - .builtin_call_comma, - .builtin_call_two, - .builtin_call_two_comma, - => { + .builtin_call_two, .builtin_call_two_comma => { const builtin_token = main_tokens[node]; const builtin_name = tree.tokenSlice(builtin_token); // If the builtin is an invalid name, we don't cause an error here; instead // let it pass, and the error will be "invalid builtin function" later. const builtin_info = BuiltinFn.list.get(builtin_name) orelse return false; - return builtin_info.needs_mem_loc; + switch (builtin_info.needs_mem_loc) { + .never => return false, + .always => return true, + .forward1 => node = node_datas[node].rhs, + } + }, + + .builtin_call, .builtin_call_comma => { + const params = tree.extra_data[node_datas[node].lhs..node_datas[node].rhs]; + const builtin_token = main_tokens[node]; + const builtin_name = tree.tokenSlice(builtin_token); + // If the builtin is an invalid name, we don't cause an error here; instead + // let it pass, and the error will be "invalid builtin function" later. + const builtin_info = BuiltinFn.list.get(builtin_name) orelse return false; + switch (builtin_info.needs_mem_loc) { + .never => return false, + .always => return true, + .forward1 => node = params[1], + } }, } } diff --git a/src/BuiltinFn.zig b/src/BuiltinFn.zig index 8f23ec86d7..e1f4f5bd16 100644 --- a/src/BuiltinFn.zig +++ b/src/BuiltinFn.zig @@ -110,10 +110,19 @@ pub const Tag = enum { Vector, }; +pub const MemLocRequirement = enum { + /// The builtin never needs a memory location. + never, + /// The builtin always needs a memory location. + always, + /// The builtin forwards the question to argument at index 1. + forward1, +}; + tag: Tag, -/// `true` if the builtin call can take advantage of a result location pointer. -needs_mem_loc: bool = false, +/// Info about the builtin call's ability to take advantage of a result location pointer. +needs_mem_loc: MemLocRequirement = .never, /// `true` if the builtin call can be the left-hand side of an expression (assigned to). allows_lvalue: bool = false, /// The number of parameters to this builtin function. `null` means variable number @@ -148,7 +157,7 @@ pub const list = list: { "@as", .{ .tag = .as, - .needs_mem_loc = true, + .needs_mem_loc = .forward1, .param_count = 2, }, }, @@ -184,7 +193,7 @@ pub const list = list: { "@bitCast", .{ .tag = .bit_cast, - .needs_mem_loc = true, + .needs_mem_loc = .forward1, .param_count = 2, }, }, @@ -248,7 +257,7 @@ pub const list = list: { "@call", .{ .tag = .call, - .needs_mem_loc = true, + .needs_mem_loc = .always, .param_count = 3, }, }, @@ -410,7 +419,7 @@ pub const list = list: { "@field", .{ .tag = .field, - .needs_mem_loc = true, + .needs_mem_loc = .always, .param_count = 2, .allows_lvalue = true, }, @@ -699,7 +708,6 @@ pub const list = list: { "@splat", .{ .tag = .splat, - .needs_mem_loc = true, .param_count = 2, }, }, @@ -714,7 +722,7 @@ pub const list = list: { "@src", .{ .tag = .src, - .needs_mem_loc = true, + .needs_mem_loc = .always, .param_count = 0, }, }, @@ -869,7 +877,7 @@ pub const list = list: { "@unionInit", .{ .tag = .union_init, - .needs_mem_loc = true, + .needs_mem_loc = .always, .param_count = 3, }, },