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

Relax constraints on inlining for some single calls #43113

Merged
merged 4 commits into from
Nov 23, 2021

Conversation

ianatol
Copy link
Member

@ianatol ianatol commented Nov 17, 2021

Fixes #43104

I'm not sure if this is the correct condition, but it seems to pass all relevant tests. My first real look into the inlining logic, so I could certainly be missing something here.

CC @aviatesk

@ianatol ianatol requested a review from aviatesk November 17, 2021 03:28
test/compiler/inline.jl Outdated Show resolved Hide resolved
test/compiler/inline.jl Outdated Show resolved Hide resolved
test/compiler/inline.jl Outdated Show resolved Hide resolved
test/compiler/inline.jl Outdated Show resolved Hide resolved
base/compiler/ssair/inlining.jl Outdated Show resolved Hide resolved
@aviatesk aviatesk added the compiler:optimizer Optimization passes (mostly in base/compiler/ssair/) label Nov 17, 2021
@ianatol ianatol marked this pull request as draft November 17, 2021 16:19
base/compiler/ssair/inlining.jl Outdated Show resolved Hide resolved
test/compiler/inline.jl Outdated Show resolved Hide resolved
test/compiler/inline.jl Outdated Show resolved Hide resolved
test/compiler/inline.jl Outdated Show resolved Hide resolved
@aviatesk
Copy link
Member

We can also handle constant-prop'ed callsites.
If we implement the similar enhancement within maybe_handle_const_call! as well, I think we can also pass the following test case:

@inline isGoodType2(cnd, @nospecialize x::Type) =
    x !== Any && !(@noinline (cnd ? Core.Compiler.isType : Base.has_free_typevars)(x))
let # aggressive inlining of single, abstract method match (with constant-prop'ed reuslt)
    src = code_typed((Type, Any,)) do x, y
        isGoodType2(true, x), isGoodType2(true, y)
    end |> only |> first
    # both callsites should be inlined (with constant-prop'ed result)
    @test count(isinvoke(:isType), src.code) == 2
    @test count(isinvoke(:has_free_typevars), src.code) == 0
    # `isGoodType2(y::Any)` isn't fully convered, thus a runtime type check and fallback dynamic dispatch should be inserted
    @test count(iscall((src,isGoodType2)), src.code) == 1
end

@aviatesk aviatesk marked this pull request as ready for review November 17, 2021 18:40
item = analyze_method!(match, argtypes, state, flag)
item === nothing && return
push!(cases, InliningCase(match.spec_types, item))
fully_covered = true
Copy link
Member

Choose a reason for hiding this comment

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

Isn't fully_covered already simply a field of MethodMatch?

Copy link
Member

Choose a reason for hiding this comment

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

ah, yeah I agree that we can just use it for the single, non-isdispatchtuple case.

@vtjnash vtjnash requested a review from Keno November 17, 2021 20:22
@aviatesk
Copy link
Member

Rebased and added the enhancement for constant-prop' handling case too.
I think this is ready for merge.

@aviatesk
Copy link
Member

This seems to certainly eliminate some runtime dispatches:

before

    JULIA usr/lib/julia/sys-o.a
Generating REPL precompile statements... 36/36
Executing precompile statements... 1451/1486
Precompilation complete. Summary:
Total ─────── 115.295423 seconds
Generation ──  88.625775 seconds 76.8684%
Execution ───  26.669648 seconds 23.1316%

after

    JULIA usr/lib/julia/sys-o.a
Generating REPL precompile statements... 36/36
Executing precompile statements... 1424/1459
Precompilation complete. Summary:
Total ─────── 113.570618 seconds
Generation ──  86.702678 seconds 76.3425%
Execution ───  26.867939 seconds 23.6575%

@aviatesk aviatesk merged commit 3f3632c into JuliaLang:master Nov 23, 2021
aviatesk added a commit that referenced this pull request Feb 13, 2022
After #43113 Julia compiler can inline `@nospecialize ::AbstractType`
signature, so we can reintroduce the input type annotations.
Still I want to keep the current `::Any` signature for those utility
functions heavily in `Core.Compiler` (e.g. `isbitstype`) because `isa(t, Type)`
check inserted by the inliner otherwise might involve some cost.
But I agree that the other non-performance sensitive functions like `which`
is better to have input type restrictions.
DilumAluthge pushed a commit that referenced this pull request Feb 13, 2022
After #43113 Julia compiler can inline `@nospecialize ::AbstractType`
signature, so we can reintroduce the input type annotations.
Still I want to keep the current `::Any` signature for those utility
functions heavily in `Core.Compiler` (e.g. `isbitstype`) because `isa(t, Type)`
check inserted by the inliner otherwise might involve some cost.
But I agree that the other non-performance sensitive functions like `which`
is better to have input type restrictions.
antoine-levitt pushed a commit to antoine-levitt/julia that referenced this pull request Feb 17, 2022
…Lang#44155)

After JuliaLang#43113 Julia compiler can inline `@nospecialize ::AbstractType`
signature, so we can reintroduce the input type annotations.
Still I want to keep the current `::Any` signature for those utility
functions heavily in `Core.Compiler` (e.g. `isbitstype`) because `isa(t, Type)`
check inserted by the inliner otherwise might involve some cost.
But I agree that the other non-performance sensitive functions like `which`
is better to have input type restrictions.
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
Co-authored-by: Shuhei Kadowaki <aviatesk@gmail.com>
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
…Lang#44155)

After JuliaLang#43113 Julia compiler can inline `@nospecialize ::AbstractType`
signature, so we can reintroduce the input type annotations.
Still I want to keep the current `::Any` signature for those utility
functions heavily in `Core.Compiler` (e.g. `isbitstype`) because `isa(t, Type)`
check inserted by the inliner otherwise might involve some cost.
But I agree that the other non-performance sensitive functions like `which`
is better to have input type restrictions.
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
Co-authored-by: Shuhei Kadowaki <aviatesk@gmail.com>
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
…Lang#44155)

After JuliaLang#43113 Julia compiler can inline `@nospecialize ::AbstractType`
signature, so we can reintroduce the input type annotations.
Still I want to keep the current `::Any` signature for those utility
functions heavily in `Core.Compiler` (e.g. `isbitstype`) because `isa(t, Type)`
check inserted by the inliner otherwise might involve some cost.
But I agree that the other non-performance sensitive functions like `which`
is better to have input type restrictions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:optimizer Optimization passes (mostly in base/compiler/ssair/)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

more aggressive inlining of single, abstract method match
3 participants