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

Add code_ircode #45306

Merged
merged 11 commits into from
May 23, 2022
Merged

Add code_ircode #45306

merged 11 commits into from
May 23, 2022

Conversation

tkf
Copy link
Member

@tkf tkf commented May 13, 2022

When working on code like #45305, it would be helpful if we can easily get "real" IRCode examples. It's also useful for writing tests. So, I wonder if we can have a code_typed-like API for IRCode?

@tkf tkf requested review from aviatesk, vtjnash and vchuravy May 13, 2022 13:25
@gbaraldi
Copy link
Member

This looks very useful. Your ShowCode.jl package has been very nice for working at the IRCode level

base/compiler/typeinfer.jl Outdated Show resolved Hide resolved
base/compiler/typeinfer.jl Outdated Show resolved Hide resolved
base/compiler/typeinfer.jl Outdated Show resolved Hide resolved
tkf and others added 2 commits May 17, 2022 01:19
Co-authored-by: Shuhei Kadowaki <40514306+aviatesk@users.noreply.github.com>
@tkf tkf force-pushed the typeinf-ircode branch from b7e3388 to d5a9dc2 Compare May 17, 2022 06:08
@aviatesk
Copy link
Member

A very exciting showcase of this PR!: https://nbviewer.org/gist/tkf/d4734be24d2694a3afd669f8f50e6b0f/00_notebook.ipynb

@tkf
Copy link
Member Author

tkf commented May 18, 2022

I guess you mean #45305 :) Well, it's also true that the notebook is also a showcase of this PR, in the sense that I can (also want to) re-write ShowCode based on this patch.

Also, FYI, I'll probably tweak the API a little bit. So, the exact API may be a bit different by the time we merge #45305. But it'll have a similar flavor.

To match `typeinf_ircode` with how typeinf lock is used ATM (i.e.,
optimizer is run inside the lock), we can manually lock it because the
lock is re-entrant.
@tkf
Copy link
Member Author

tkf commented May 18, 2022

@aviatesk @ianatol I think all issues are addressed. Shall we merge this?

Copy link
Member

@aviatesk aviatesk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, let's go ahead!

@ianatol
Copy link
Member

ianatol commented May 18, 2022

Yup, looks good!

base/reflection.jl Outdated Show resolved Hide resolved
"""
function run_minimal_passes(ci::CodeInfo, sv::OptimizationState, caller::InferenceResult)
@timeit "convert" ir = convert_to_ircode(ci, sv)
@timeit "slot2reg" ir = slot2reg(ir, ci, sv)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not consider this to be a minimal pass, since it is quite a significant optimization and change to the IR

Suggested change
@timeit "slot2reg" ir = slot2reg(ir, ci, sv)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be very rare to deal with IRCode without slog2reg (unless you are changing slog2reg itself). For one thing, verify_ir considers IRCode with slots invalid:

elseif isa(op, Union{SlotNumber, TypedSlot})
@verify_error "Left over slot detected in converted IR"

So, I think running slot2reg with optimize = false makes sense, if we treat it as "least optimized typical IRCode that compiler passes deal with." But running slot2reg manually is not super hard. So I don't have super strong opinion about it.

@aviatesk @ianatol Any thoughts?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we can also pass numeric optimize_level

function run_passes(ci::CodeInfo, sv::OptimizationState, caller::InferenceResult, optimize_level = typeinf(Int))
    stage = 1
    isdone() = optimize_level < (stage += 1)
    @timeit "convert"   ir = convert_to_ircode(ci, sv)
    isdone() && return ir
    @timeit "slot2reg"  ir = slot2reg(ir, ci, sv)
    isdone() && return ir
    # TODO: Domsorting can produce an updated domtree - no need to recompute here
    @timeit "compact 1" ir = compact!(ir)
    isdone() && return ir
    @timeit "Inlining"  ir = ssa_inlining_pass!(ir, ir.linetable, sv.inlining, ci.propagate_inbounds)
    isdone() && return ir
    ...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer running slot2reg as we often work on post-slot2reg IRCode when developing Julia-level optimizer. I also think the other "pre-inlining optimizations (mostly DCE)" happening within convert_to_ircode and type_annotate! is also significant anyway.

And the optimize_level idea sounds very useful!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added optimize_level 04797d5

julia> function demo(f)
           y = f()
           y isa Any ? nothing : f()
       end;

julia> Base.code_ircode(demo; optimize_level = 1) |> only |> first
2 1 ─      (_3 = (_2)())::#UNDEF                                                                   │
3%2 = (_3 isa Main.Any)::#UNDEF                                                               │
  └──      goto #5 if not %2::#UNDEF                                                               │
  2return Main.nothing::#UNDEF                                                             │
  3 ─      Core.Const(:((_2)()))::#UNDEF                                                           │
  │        Core.Const(:(return %5))::#UNDEF                                                        │
  └──      unreachable::#UNDEF                                                                     │


julia> Base.code_ircode(demo; optimize_level = 2) |> only |> first
2 1%1 = (_2)()::Any3%2 = (%1 isa Main.Any)::Core.Const(true)                                                     │
  └──      goto #3 if not %2                                                                       │
  2return Main.nothing3 ─      Core.Const(:((_2)()))::Union{}                                                          │
  │        Core.Const(:(return %5))::Union{}                                                       │
  └──      unreachable                                                                             │


julia> Base.code_ircode(demo) |> only |> first
2 1 ─     (_2)()::Any3 └──     goto #3 if not true                                                                      │
  2return Main.nothing3 ─     unreachable                                                                              │

base/compiler/optimize.jl Outdated Show resolved Hide resolved
tkf and others added 3 commits May 20, 2022 02:26
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@aviatesk
Copy link
Member

Just in case: @nanosoldier runbenchmarks("inference", vs=":master")

@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - no performance regressions were detected. A full report can be found here.

@tkf tkf requested a review from vtjnash May 20, 2022 13:31
@tkf
Copy link
Member Author

tkf commented May 20, 2022

Good to go?

@vtjnash Do you want to review this?

@aviatesk aviatesk merged commit 5e1c5cf into JuliaLang:master May 23, 2022
pchintalapudi pushed a commit that referenced this pull request May 25, 2022
To match `typeinf_ircode` with how typeinf lock is used ATM (i.e.,
optimizer is run inside the lock), we can manually lock it because the
lock is re-entrant.

Co-authored-by: Shuhei Kadowaki <aviatesk@gmail.com>
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.

None yet

7 participants