Skip to content

Commit 7826d5c

Browse files
topolarityKristofferC
authored andcommitted
[release-1.12] Workaround poor inlining behavior for if @generated 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 ```
1 parent 227ff67 commit 7826d5c

File tree

2 files changed

+36
-1
lines changed

2 files changed

+36
-1
lines changed

Compiler/src/tfuncs.jl

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,13 @@ function add_tfunc(@nospecialize(f::Builtin), minarg::Int, maxarg::Int, @nospeci
8888
end
8989

9090
add_tfunc(throw, 1, 1, @nospecs((𝕃::AbstractLattice, x)->Bottom), 0)
91-
add_tfunc(Core.throw_methoderror, 1, INT_INF, @nospecs((𝕃::AbstractLattice, x)->Bottom), 0)
91+
92+
# FIXME: the inlining cost for this built-in should be 0 (just like throw), but it is set to
93+
# 1000 to avoid regressions associated with the Compiler inlining too eagerly, especially when
94+
# encounting `if @generated` expressions.
95+
#
96+
# c.f. https://github.com/JuliaLang/julia/issues/58915#issuecomment-3070231392
97+
add_tfunc(Core.throw_methoderror, 1, INT_INF, @nospecs((𝕃::AbstractLattice, x)->Bottom), 1000)
9298

9399
# the inverse of typeof_tfunc
94100
# returns (type, isexact, isconcrete, istype)

Compiler/test/inline.jl

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2310,3 +2310,32 @@ g_noinline_invoke(x) = f_noinline_invoke(x)
23102310
let src = code_typed1(g_noinline_invoke, (Union{Symbol,Nothing},))
23112311
@test !any(@nospecialize(x)->isa(x,GlobalRef), src.code)
23122312
end
2313+
2314+
# https://github.com/JuliaLang/julia/issues/58915
2315+
f58915(nt) = @inline Base.setindex(nt, 2, :next)
2316+
# This function should fully-inline, i.e. it should have only built-in / intrinsic calls
2317+
# and no invokes or dynamic calls of user code
2318+
let src = code_typed(f58915, Tuple{@NamedTuple{next::UInt32,prev::UInt32}})[1].first
2319+
# Any calls should be built-in calls
2320+
@test count(iscall(f->!isa(singleton_type(argextype(f, src)), Core.Builtin)), src.code) == 0
2321+
# There should be no invoke at all
2322+
@test count(isinvoke(Returns(true)), src.code) == 0
2323+
end
2324+
2325+
# https://github.com/JuliaLang/julia/issues/58915#issuecomment-3061421895
2326+
let src = code_typed(Base.setindex, (@NamedTuple{next::UInt32,prev::UInt32}, Int, Symbol))[1].first
2327+
@test count(isinvoke(:merge_fallback), src.code) == 0
2328+
@test count(iscall((src, Base.merge_fallback)), src.code) == 0
2329+
end
2330+
2331+
# https://github.com/JuliaLang/julia/pull/58996#issuecomment-3073496955
2332+
f58996(::Int) = :int
2333+
f58996(::String) = :string
2334+
call_f58996(x) = f58996(x)
2335+
callcall_f58996(x) = call_f58996(x);
2336+
let src = code_typed(callcall_f58996, (Any,))[1].first
2337+
# Any calls should be built-in calls
2338+
@test_broken count(iscall(f->!isa(singleton_type(argextype(f, src)), Core.Builtin)), src.code) == 0
2339+
# There should be no invoke at all
2340+
@test count(isinvoke(Returns(true)), src.code) == 0
2341+
end

0 commit comments

Comments
 (0)