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

Simplify building the @generated function by returning the CodeInfo #8

Merged
merged 1 commit into from
Oct 23, 2019

Conversation

NHDaly
Copy link
Owner

@NHDaly NHDaly commented Oct 16, 2019

Apparently using an actual return code_info line, instead of just having the code_info as the final expression, prevents Julia lowering pass from wrapping the return value in an Expr(:block, ...), meaning it can be used by Julia to build the generated function.

Here is an example showing that difference:

With return:

julia> Meta.@lower @generated function f() return 2 end
...
    $(Expr(:meta, :nospecialize, :(@_2)))
    return 2
    Core._expr(:block, $(QuoteNode(:(#= REPL[107]:1 =#))), nothing)
    return %3
...

Without return:

julia> Meta.@lower @generated function f() 2 end
...
    $(Expr(:meta, :nospecialize, :(@_2)))
    Core._expr(:block, $(QuoteNode(:(#= REPL[108]:1 =#))), 2)
    return %2
...

As far as I can tell, it's been like this since Julia 0.7.

Before this PR, I was manually lowering the function and unwrapping the return value to allow it to pass-through unmodified. Now, this syntax change somehow stops the lowering-pass from wrapping it in the first place.


That said, this seems like maybe a bug in Julia's lowering logic... I'm leaving a big note in the code, and we may need to re-evaluate this if that lowering logic changes in the future.

@NHDaly
Copy link
Owner Author

NHDaly commented Oct 16, 2019

Haha, this PR is so ridiculous. CC: @vchuravy @oxinabox

@NHDaly NHDaly merged commit 6fafbc5 into master Oct 23, 2019
@NHDaly NHDaly deleted the simplify_make_generated--return branch October 23, 2019 16:34
@vchuravy
Copy link
Collaborator

That said, this seems like maybe a bug in Julia's lowering logic... I'm leaving a big note in the code, and we may need to re-evaluate this if that lowering logic changes in the future.

Maybe ask Jeff whether this is intended :P

@oxinabox
Copy link

It certainly took me a back when I realized that was the difference between my code and @NHDaly's

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants