Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: introduce new RLS mechanism to share peers #20627

Open
mlugg opened this issue Jul 14, 2024 · 0 comments
Open

Proposal: introduce new RLS mechanism to share peers #20627

mlugg opened this issue Jul 14, 2024 · 0 comments
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@mlugg
Copy link
Member

mlugg commented Jul 14, 2024

This is a proposal that's been cooking in the back of my mind for some time, and I think it's finally fully-formed.

Background

In Zig, we refer to a set of expressions as being "peers" if Peer Type Resolution is applied to the set. For example, consider the following code:

const x = switch (my_u8) {
    0 => my_u32,
    1 => 10,
    else => 20,
};

Here, the expressions my_u32, 10, and 20 are peers. This is what allows this code to work correctly at runtime despite 10 and 20 having type comptime_int: Peer Type Resolution resolves that these expressions must all have type u32 (the peer type of u32, comptime_int, comptime_int), so the compiler inserts code to coerce them each to u32.

Unfortunately, the peer mechanism is fairly limited. In a sense, peers do not "nest well". For instance, consider this code:

const x = if (my_u8 == 0) my_u32 else if (my_u8 == 1) 10 else 20;

One might expect this code to act identically to the switch expression above. However, at runtime, this causes a compile error. This is because chained else-if expressions are not a first-class concept in Zig, but rather nested expressions; so, this expression parses as if (my_u8 == 0) my_u32 else (if (my_u8 == 1) 10 else 20). The inner if expression here has two comptime_int peers, so PTR determines the expression as a whole to have type comptime_int, meaning a value of a comptime-only type depends on runtime control flow.

This isn't a huge issue, but can be unintuitive and frustrating in some cases. Notably, #11957 actually special-cased a few syntax forms with the explicit goal of making sub-expressions peers when they otherwise would not be. This compiler change had other benefits, but this language change was the primary motivator. The problem with this is that it subtly complicates the language specification, and leads to somewhat unintuitive behavior where seemingly-equivalent code (for instance, just adding some redundant parentheses) can affect a program's behavior. Ideally, we would have a more general mechanism to achieve the same result.

Proposal

Introduce a new form of result location, which indicates that an expression's result should be a peer for a specific block. In terms of implementation, it contains the Zir.Inst.Index of a ZIR block which the result should be a break operand to.

In general, in any case where a pointer result location (std.zig.AstGen.ResultInfo.Loc.ptr) can be forwarded to sub-expressions, this result location will also be eligible for forwarding. This means that #11957 will no longer have an impact on language semantics (although the corresponding compiler changes are still valuable for optimization purposes).

Here's an example of what this allows in practice:

const x = switch (y) {
    1 => a.
    2 => if (cond) b else c,
    3 => switch (z) {
        10 => d,
        20 => e,
        else => f,
    },
    else => g,
};

Today, this overall expression performs PTR in several places:

  • b and c are peer resolved to type T1
  • d, e, and f and peer resolved to type T2
  • a, T1, T2, and g are peer resolved to the final type T of x

Under this proposal, a, b, c, d, e, f, and g would all be peers, so only one instance of peer resolution occurs. This leads to a more intuitive behavior when considering comptime-only types involved in runtime control flow.

This also has some minor performance benefits, both in the compiler itself and in generated unoptimized code. In terms of compiler performance, Sema now has to do less work overall; the code snippet above currently requires analysis of 9 break instructions and 3 peer resolutions, whereas under this proposal, it only needs analysis of 7 break instructions and 1 peer resolution. Regarding generated code, the AIR here would have a simpler structure, without multiple "layers" of br instructions appearing. It would also avoid any potential intermediate coercions -- for instance, if the final resolved type is u32, but b and c have types u8 and u16 respectively, today there would be an initial coercion of b to a u16 followed by the outer coercion to a u32, whereas under this proposal all peers are directly coerced to their final type.

@mlugg mlugg added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Jul 14, 2024
@mlugg mlugg added this to the 0.15.0 milestone Jul 14, 2024
mlugg added a commit to mlugg/zig that referenced this issue Aug 30, 2024
Also, don't use the special switch lowering for errors if the switch
is labeled; this isn't currently supported. Related: ziglang#20627.
mlugg added a commit to mlugg/zig that referenced this issue Sep 1, 2024
Also, don't use the special switch lowering for errors if the switch
is labeled; this isn't currently supported. Related: ziglang#20627.
mlugg added a commit to mlugg/zig that referenced this issue Sep 1, 2024
Also, don't use the special switch lowering for errors if the switch
is labeled; this isn't currently supported. Related: ziglang#20627.
richerfu pushed a commit to richerfu/zig that referenced this issue Oct 28, 2024
Also, don't use the special switch lowering for errors if the switch
is labeled; this isn't currently supported. Related: ziglang#20627.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

1 participant