[release-1.12] Workaround poor inlining behavior for if @generated functions#58996
Merged
topolarity merged 2 commits intoJuliaLang:backports-release-1.12from Jul 15, 2025
Merged
Conversation
Restores something very close to the previous inlining behavior, without reverting the change to statically-encode `MethodError`s in the IR. This built-in should not actually have a high cost, but it coincidentally encourages our compiler not to inline wide calls to generators that it will never be able to refine to the IR / type-information it would have gotten by exploring the more narrow call to begin with.
Member
|
This sounded more like a bug with compute_sparams cost? We know that function is much more expensive than dynamic dispatch, so it should never inline unless the user explicitly overrides the heuristics |
Member
Author
It probably is, but I wasn't sure if there isn't also arguably a separate bug around whether we should inline In any case, this PR is just a "spiritual revert" to get back to the old behavior before we get the proper fix(es) in |
Member
|
I think this revert is fine. Having said that now inlining won't work in cases like the one below for example, so I still think it would be better to fix the root problem if possible. julia> func(::Int) = :int;
julia> func(::String) = :string;
julia> callfunc(x) = func(x);
julia> callcallfunc(x) = callfunc(x);
julia> code_typed(callcallfunc, (Any,))
1-element Vector{Any}:
CodeInfo(
1 ─ %1 = dynamic Main.callfunc(x)::Symbol
└── return %1
) => Symbol |
aviatesk
reviewed
Jul 15, 2025
Member
|
I made #59013 based on #58996 (comment). Does that seem like a better approach than this PR? |
Member
Author
|
I'm going to merge this (just so that we have an easy-to-justify fix on 1.12) but I like the direction you're taking in #59013 @serenity4 |
3960ef4
into
JuliaLang:backports-release-1.12
6 of 7 checks passed
DelveCI
pushed a commit
to RelationalAI/julia
that referenced
this pull request
Jul 23, 2025
…functions (JuliaLang#58996) Restores something very close to the previous inlining behavior, without reverting JuliaLang#54972 This is a hack to workaround JuliaLang#58915 (comment) for 1.12, but we should leave an issue open so that we can put in a proper fix to inlining for the next major version. Also improves JuliaLang#58915 (comment), which was a dynamic `call` on 1.11 and a poorly-chosen `invoke` of the generator fallback on 1.12. This is now an inlined version of the non-fallback path for the generator: ```julia julia> nt = (next = zero(UInt32), prev = zero(UInt32)) (next = 0x00000000, prev = 0x00000000) julia> f(nt) = @inline Base.setindex(nt, 2, :next) f (generic function with 1 method) julia> @code_warntype optimize=true f(nt) MethodInstance for f(::@NamedTuple{next::UInt32, prev::UInt32}) from f(nt) @ Main REPL[2]:1 Arguments #self#::Core.Const(Main.f) nt::@NamedTuple{next::UInt32, prev::UInt32} Body::@NamedTuple{next::Int64, prev::UInt32} 1 ─ %1 = builtin Base.getfield(nt, :prev)::UInt32 │ %2 = %new(@NamedTuple{next::Int64, prev::UInt32}, 2, %1)::@NamedTuple{next::Int64, prev::UInt32} └── return %2 ```
DelveCI
pushed a commit
to RelationalAI/julia
that referenced
this pull request
Jul 26, 2025
…functions (JuliaLang#58996) Restores something very close to the previous inlining behavior, without reverting JuliaLang#54972 This is a hack to workaround JuliaLang#58915 (comment) for 1.12, but we should leave an issue open so that we can put in a proper fix to inlining for the next major version. Also improves JuliaLang#58915 (comment), which was a dynamic `call` on 1.11 and a poorly-chosen `invoke` of the generator fallback on 1.12. This is now an inlined version of the non-fallback path for the generator: ```julia julia> nt = (next = zero(UInt32), prev = zero(UInt32)) (next = 0x00000000, prev = 0x00000000) julia> f(nt) = @inline Base.setindex(nt, 2, :next) f (generic function with 1 method) julia> @code_warntype optimize=true f(nt) MethodInstance for f(::@NamedTuple{next::UInt32, prev::UInt32}) from f(nt) @ Main REPL[2]:1 Arguments #self#::Core.Const(Main.f) nt::@NamedTuple{next::UInt32, prev::UInt32} Body::@NamedTuple{next::Int64, prev::UInt32} 1 ─ %1 = builtin Base.getfield(nt, :prev)::UInt32 │ %2 = %new(@NamedTuple{next::Int64, prev::UInt32}, 2, %1)::@NamedTuple{next::Int64, prev::UInt32} └── return %2 ```
KristofferC
pushed a commit
that referenced
this pull request
Jul 27, 2025
…functions (#58996) Restores something very close to the previous inlining behavior, without reverting #54972 This is a hack to workaround #58915 (comment) for 1.12, but we should leave an issue open so that we can put in a proper fix to inlining for the next major version. Also improves #58915 (comment), which was a dynamic `call` on 1.11 and a poorly-chosen `invoke` of the generator fallback on 1.12. This is now an inlined version of the non-fallback path for the generator: ```julia julia> nt = (next = zero(UInt32), prev = zero(UInt32)) (next = 0x00000000, prev = 0x00000000) julia> f(nt) = @inline Base.setindex(nt, 2, :next) f (generic function with 1 method) julia> @code_warntype optimize=true f(nt) MethodInstance for f(::@NamedTuple{next::UInt32, prev::UInt32}) from f(nt) @ Main REPL[2]:1 Arguments #self#::Core.Const(Main.f) nt::@NamedTuple{next::UInt32, prev::UInt32} Body::@NamedTuple{next::Int64, prev::UInt32} 1 ─ %1 = builtin Base.getfield(nt, :prev)::UInt32 │ %2 = %new(@NamedTuple{next::Int64, prev::UInt32}, 2, %1)::@NamedTuple{next::Int64, prev::UInt32} └── return %2 ```
KristofferC
pushed a commit
that referenced
this pull request
Sep 15, 2025
…functions (#58996) Restores something very close to the previous inlining behavior, without reverting #54972 This is a hack to workaround #58915 (comment) for 1.12, but we should leave an issue open so that we can put in a proper fix to inlining for the next major version. Also improves #58915 (comment), which was a dynamic `call` on 1.11 and a poorly-chosen `invoke` of the generator fallback on 1.12. This is now an inlined version of the non-fallback path for the generator: ```julia julia> nt = (next = zero(UInt32), prev = zero(UInt32)) (next = 0x00000000, prev = 0x00000000) julia> f(nt) = @inline Base.setindex(nt, 2, :next) f (generic function with 1 method) julia> @code_warntype optimize=true f(nt) MethodInstance for f(::@NamedTuple{next::UInt32, prev::UInt32}) from f(nt) @ Main REPL[2]:1 Arguments #self#::Core.Const(Main.f) nt::@NamedTuple{next::UInt32, prev::UInt32} Body::@NamedTuple{next::Int64, prev::UInt32} 1 ─ %1 = builtin Base.getfield(nt, :prev)::UInt32 │ %2 = %new(@NamedTuple{next::Int64, prev::UInt32}, 2, %1)::@NamedTuple{next::Int64, prev::UInt32} └── return %2 ```
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Restores something very close to the previous inlining behavior, without reverting #54972
This is a hack to workaround #58915 (comment) for 1.12, but we should leave an issue open so that we can put in a proper fix to inlining for the next major version.
Also improves #58915 (comment), which was a dynamic
callon 1.11 and a poorly-choseninvokeof the generator fallback on 1.12. This is now an inlined version of the non-fallback path for the generator: