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

defer implementation: instead of duplicating the code at every exit path, make a basic block that exit paths point to #283

Open
andrewrk opened this issue Mar 26, 2017 · 7 comments
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. frontend Tokenization, parsing, AstGen, Sema, and Liveness. optimization proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@andrewrk
Copy link
Member

From #110

@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. optimization labels Mar 26, 2017
@andrewrk andrewrk modified the milestone: 0.3.0 May 20, 2017
@andrewrk andrewrk modified the milestones: 0.3.0, 0.4.0 Feb 28, 2018
@andrewrk andrewrk modified the milestones: 0.4.0, 0.5.0 Feb 22, 2019
@shawnl
Copy link
Contributor

shawnl commented Apr 6, 2019

That's what __attribute__((cleanup)) does in C code.

@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Aug 22, 2019
@andrewrk andrewrk added the stage1 The process of building from source via WebAssembly and the C backend. label Feb 10, 2020
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Feb 10, 2020
@Sircular
Copy link
Contributor

I'm not sure how this would work inside loops, but I'm guessing that outside of loops this would require a jump to a point near the end of the function. Is goto implemented in Zig IR? I haven't seen it, and that would be the most straightforward way to implement this, but I might have just missed it.

@andrewrk
Copy link
Member Author

andrewrk commented May 23, 2020

Yes goto is available in zig ir

@andrewrk andrewrk added frontend Tokenization, parsing, AstGen, Sema, and Liveness. and removed stage1 The process of building from source via WebAssembly and the C backend. labels Jul 30, 2020
@andrewrk
Copy link
Member Author

I no longer think this is important to do in stage1. Here's a proposal for how to make this work in stage2:

zig code

fn entry(cond: bool) i32 {
    defer foo();
    {
        defer bar();
        if (cond) {
            return 10;
        }
        baz();
    }
    quux();
    return ret();
}

ZIR

fn(@fnty, {
    %ret_flag = alloc(bool)
    %18 = store(%ret_flag, false)
    %0 = block(defer_foo, {
        %14 = block(user, {
            %4 = block(defer_bar, {
                %6 = block(if, {
                    %7 = condbr(%arg0, {
                        %16 = int(10)
                        %17 = set_ret_val(%16)
                        %19 = store(%ret_flag, true)
                        %8 = break_void(defer_bar)
                    }, {
                        %11 = break_void(if)
                    })
                })
                %12 = call(baz, [])
                %10 = break_void(defer_bar)
            })
            %5 = call(bar, [])
            %21 = load(%ret_flag)
            %9 = cond_break_void(%21, defer_foo, user)
        }
        %13 = call(quux, [])
        %2 = call(ret, [])
        %3 = set_ret_val(%2)
        %20 = store(%ret_flag, true)
        %14 = break_void(defer_foo)
    })
    %1 = call(foo, [])
})

So the idea here is to create a block for every defer expression. The defer expression goes after the block ends, which makes it run unconditionally upon exiting the block. Here I've renamed brvoid to break_void for clarity, and introduced cond_break_void which is the same thing except has a boolean condition that decides which block to break from.

At the very end, the "return" from the function is implied.

You can probably spot some easy optimizations to do here. Sometimes blocks could be elided, and defer expressions that occur in a row could be put into the same block.

I should come up with an example with errdefer but it does work nicely with this. The %ret_flag turns into an enum instead of a bool, and the cond_break_void turns into a switch_break_void with the 3 possibilities being enum { not_returning, return_with_error, return_with_payload }, and depending on the tag, control flow will proceed to, respectively, the next block in scope, the next defer (including errdefers), or the next non-err-defer.

@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 9, 2020
@andrewrk andrewrk added optimization proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. labels Apr 21, 2021
@andrewrk andrewrk modified the milestones: 0.8.0, 1.0.0 Apr 21, 2021
@andrewrk
Copy link
Member Author

I spent some time on this. Here are some points:

  • It is not obvious that one way or the other will be better in terms of performance. Empirically, LLVM has no problem optimizing status quo (duplicated expressions). The alternative presented here has not been tested.
  • The duplicated expressions strategy (status quo) is simpler to implement.

Therefore I will stick with the duplicated expressions strategy, and mark this issue as an optimization, that needs to be investigated before it is accepted as the plan for the future.

@LemonBoy
Copy link
Contributor

The duplicated-expression approach can be seen as an optimized version of the multiple-block one where you're inlining every single block.
IMO it makes sense to implement the block approach and allow LLVM but also the Zig frontend to eventually inline them if needed, eg. depending on the optimization flags.
If you consider a function with a decent number of exit points the block-approach produces much smaller code and so it's better suited for ReleaseSize, while for ReleaseFast we may get better performance by inlining the sequence at every callsite provided it doesn't expand to too many ZIR instructions.

@lerno
Copy link

lerno commented Mar 31, 2022

What about static variables in the duplicated expression strategy? Does Zig currently correctly handle that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. frontend Tokenization, parsing, AstGen, Sema, and Liveness. optimization 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

5 participants