Skip to content

Commit 51f1df2

Browse files
author
KristofferC
committed
Revert "improve performance issue of @nospecialize-d keyword func call (#47059)"
This reverts commit 95cfd62.
1 parent a16ffd6 commit 51f1df2

File tree

8 files changed

+15
-100
lines changed

8 files changed

+15
-100
lines changed

base/boot.jl

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -620,8 +620,7 @@ end
620620

621621
NamedTuple() = NamedTuple{(),Tuple{}}(())
622622

623-
eval(Core, :(NamedTuple{names}(args::Tuple) where {names} =
624-
$(Expr(:splatnew, :(NamedTuple{names,typeof(args)}), :args))))
623+
NamedTuple{names}(args::Tuple) where {names} = NamedTuple{names,typeof(args)}(args)
625624

626625
using .Intrinsics: sle_int, add_int
627626

base/compiler/abstractinterpretation.jl

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2179,17 +2179,17 @@ function abstract_eval_statement_expr(interp::AbstractInterpreter, e::Expr, vtyp
21792179
merge_effects!(interp, sv, effects)
21802180
elseif ehead === :splatnew
21812181
t, isexact = instanceof_tfunc(abstract_eval_value(interp, e.args[1], vtypes, sv))
2182-
nothrow = false # TODO: More precision
2183-
if length(e.args) == 2 && isconcretedispatch(t) && !ismutabletype(t)
2182+
is_nothrow = false # TODO: More precision
2183+
if length(e.args) == 2 && isconcretetype(t) && !ismutabletype(t)
21842184
at = abstract_eval_value(interp, e.args[2], vtypes, sv)
21852185
n = fieldcount(t)
21862186
if isa(at, Const) && isa(at.val, Tuple) && n == length(at.val::Tuple) &&
2187-
let t = t, at = at; all(i::Int->getfield(at.val::Tuple, i) isa fieldtype(t, i), 1:n); end
2188-
nothrow = isexact
2187+
let t = t, at = at; _all(i->getfield(at.val::Tuple, i) isa fieldtype(t, i), 1:n); end
2188+
is_nothrow = isexact && isconcretedispatch(t)
21892189
t = Const(ccall(:jl_new_structt, Any, (Any, Any), t, at.val))
2190-
elseif isa(at, PartialStruct) && at Tuple && n == length(at.fields::Vector{Any}) &&
2191-
let t = t, at = at; all(i::Int->(at.fields::Vector{Any})[i] fieldtype(t, i), 1:n); end
2192-
nothrow = isexact
2190+
elseif isa(at, PartialStruct) && at Tuple && n == length(at.fields::Vector{Any}) &&
2191+
let t = t, at = at; _all(i->(at.fields::Vector{Any})[i] fieldtype(t, i), 1:n); end
2192+
is_nothrow = isexact && isconcretedispatch(t)
21932193
t = PartialStruct(t, at.fields::Vector{Any})
21942194
end
21952195
end

base/compiler/ssair/passes.jl

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -401,16 +401,6 @@ function lift_leaves(compact::IncrementalCompact,
401401
end
402402
lift_arg!(compact, leaf, cache_key, def, 1+field, lifted_leaves)
403403
continue
404-
# NOTE we can enable this, but most `:splatnew` expressions are transformed into
405-
# `:new` expressions by the inlinear
406-
# elseif isexpr(def, :splatnew) && length(def.args) == 2 && isa(def.args[2], AnySSAValue)
407-
# tplssa = def.args[2]::AnySSAValue
408-
# tplexpr = compact[tplssa][:inst]
409-
# if is_known_call(tplexpr, tuple, compact) && 1 ≤ field < length(tplexpr.args)
410-
# lift_arg!(compact, tplssa, cache_key, tplexpr, 1+field, lifted_leaves)
411-
# continue
412-
# end
413-
# return nothing
414404
elseif is_getfield_captures(def, compact)
415405
# Walk to new_opaque_closure
416406
ocleaf = def.args[2]
@@ -479,7 +469,7 @@ function lift_arg!(
479469
end
480470
end
481471
lifted_leaves[cache_key] = LiftedValue(lifted)
482-
return nothing
472+
nothing
483473
end
484474

485475
function walk_to_def(compact::IncrementalCompact, @nospecialize(leaf))

base/compiler/tfuncs.jl

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -407,19 +407,11 @@ add_tfunc(Core.sizeof, 1, 1, sizeof_tfunc, 1)
407407
function nfields_tfunc(@nospecialize(x))
408408
isa(x, Const) && return Const(nfields(x.val))
409409
isa(x, Conditional) && return Const(0)
410-
xt = widenconst(x)
411-
x = unwrap_unionall(xt)
410+
x = unwrap_unionall(widenconst(x))
412411
isconstType(x) && return Const(nfields(x.parameters[1]))
413412
if isa(x, DataType) && !isabstracttype(x)
414-
if x.name === Tuple.name
415-
isvatuple(x) && return Int
416-
return Const(length(x.types))
417-
elseif x.name === _NAMEDTUPLE_NAME
418-
length(x.parameters) == 2 || return Int
419-
names = x.parameters[1]
420-
isa(names, Tuple{Vararg{Symbol}}) || return nfields_tfunc(rewrap_unionall(x.parameters[2], xt))
421-
return Const(length(names))
422-
else
413+
if !(x.name === Tuple.name && isvatuple(x)) &&
414+
!(x.name === _NAMEDTUPLE_NAME && !isconcretetype(x))
423415
return Const(isdefined(x, :types) ? length(x.types) : length(x.name.names))
424416
end
425417
end
@@ -1660,12 +1652,6 @@ function apply_type_tfunc(@nospecialize(headtypetype), @nospecialize args...)
16601652
end
16611653
if istuple
16621654
return Type{<:appl}
1663-
elseif isa(appl, DataType) && appl.name === _NAMEDTUPLE_NAME && length(appl.parameters) == 2 &&
1664-
(appl.parameters[1] === () || appl.parameters[2] === Tuple{})
1665-
# if the first/second parameter of `NamedTuple` is known to be empty,
1666-
# the second/first argument should also be empty tuple type,
1667-
# so refine it here
1668-
return Const(NamedTuple{(),Tuple{}})
16691655
end
16701656
ans = Type{appl}
16711657
for i = length(outervars):-1:1

base/namedtuple.jl

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -335,27 +335,22 @@ reverse(nt::NamedTuple) = NamedTuple{reverse(keys(nt))}(reverse(values(nt)))
335335
end
336336

337337
"""
338-
structdiff(a::NamedTuple, b::Union{NamedTuple,Type{NamedTuple}})
338+
structdiff(a::NamedTuple{an}, b::Union{NamedTuple{bn},Type{NamedTuple{bn}}}) where {an,bn}
339339
340340
Construct a copy of named tuple `a`, except with fields that exist in `b` removed.
341341
`b` can be a named tuple, or a type of the form `NamedTuple{field_names}`.
342342
"""
343343
function structdiff(a::NamedTuple{an}, b::Union{NamedTuple{bn}, Type{NamedTuple{bn}}}) where {an, bn}
344344
if @generated
345345
names = diff_names(an, bn)
346-
isempty(names) && return (;) # just a fast pass
347346
idx = Int[ fieldindex(a, names[n]) for n in 1:length(names) ]
348347
types = Tuple{Any[ fieldtype(a, idx[n]) for n in 1:length(idx) ]...}
349348
vals = Any[ :(getfield(a, $(idx[n]))) for n in 1:length(idx) ]
350-
return :( NamedTuple{$names,$types}(($(vals...),)) )
349+
:( NamedTuple{$names,$types}(($(vals...),)) )
351350
else
352351
names = diff_names(an, bn)
353-
# N.B this early return is necessary to get a better type stability,
354-
# and also allows us to cut off the cost from constructing
355-
# potentially type unstable closure passed to the `map` below
356-
isempty(names) && return (;)
357352
types = Tuple{Any[ fieldtype(typeof(a), names[n]) for n in 1:length(names) ]...}
358-
return NamedTuple{names,types}(map(n::Symbol->getfield(a, n), names))
353+
NamedTuple{names,types}(map(Fix1(getfield, a), names))
359354
end
360355
end
361356

test/compiler/inference.jl

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1540,11 +1540,6 @@ end
15401540
@test nfields_tfunc(Tuple{Int, Vararg{Int}}) === Int
15411541
@test nfields_tfunc(Tuple{Int, Integer}) === Const(2)
15421542
@test nfields_tfunc(Union{Tuple{Int, Float64}, Tuple{Int, Int}}) === Const(2)
1543-
@test nfields_tfunc(@NamedTuple{a::Int,b::Integer}) === Const(2)
1544-
@test nfields_tfunc(NamedTuple{(:a,:b),T} where T<:Tuple{Int,Integer}) === Const(2)
1545-
@test nfields_tfunc(NamedTuple{(:a,:b)}) === Const(2)
1546-
@test nfields_tfunc(NamedTuple{names,Tuple{Any,Any}} where names) === Const(2)
1547-
@test nfields_tfunc(Union{NamedTuple{(:a,:b)},NamedTuple{(:c,:d)}}) === Const(2)
15481543

15491544
using Core.Compiler: typeof_tfunc
15501545
@test typeof_tfunc(Tuple{Vararg{Int}}) == Type{Tuple{Vararg{Int,N}}} where N
@@ -2369,13 +2364,6 @@ end |> only === Int
23692364
# Equivalence of Const(T.instance) and T for singleton types
23702365
@test Const(nothing) Nothing && Nothing Const(nothing)
23712366

2372-
# `apply_type_tfunc` should always return accurate result for empty NamedTuple case
2373-
import Core: Const
2374-
import Core.Compiler: apply_type_tfunc
2375-
@test apply_type_tfunc(Const(NamedTuple), Const(()), Type{T} where T<:Tuple{}) === Const(typeof((;)))
2376-
@test apply_type_tfunc(Const(NamedTuple), Const(()), Type{T} where T<:Tuple) === Const(typeof((;)))
2377-
@test apply_type_tfunc(Const(NamedTuple), Tuple{Vararg{Symbol}}, Type{Tuple{}}) === Const(typeof((;)))
2378-
23792367
# Don't pessimize apply_type to anything worse than Type and yield Bottom for invalid Unions
23802368
@test only(Base.return_types(Core.apply_type, Tuple{Type{Union}})) == Type{Union{}}
23812369
@test only(Base.return_types(Core.apply_type, Tuple{Type{Union},Any})) == Type

test/compiler/inline.jl

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1749,48 +1749,6 @@ f_ifelse_3(a, b) = Core.ifelse(a, true, b)
17491749
@test fully_eliminated(f_ifelse_2, Tuple{Any, Any}; retval=Core.Argument(3))
17501750
@test !fully_eliminated(f_ifelse_3, Tuple{Any, Any})
17511751

1752-
# inline_splatnew for abstract `NamedTuple`
1753-
@eval construct_splatnew(T, fields) = $(Expr(:splatnew, :T, :fields))
1754-
for tt = Any[(Int,Int), (Integer,Integer), (Any,Any)]
1755-
let src = code_typed1(tt) do a, b
1756-
construct_splatnew(NamedTuple{(:a,:b),typeof((a,b))}, (a,b))
1757-
end
1758-
@test count(issplatnew, src.code) == 0
1759-
@test count(isnew, src.code) == 1
1760-
end
1761-
end
1762-
1763-
# optimize away `NamedTuple`s used for handling `@nospecialize`d keyword-argument
1764-
# https://github.com/JuliaLang/julia/pull/47059
1765-
abstract type CallInfo end
1766-
struct NewInstruction
1767-
stmt::Any
1768-
type::Any
1769-
info::CallInfo
1770-
line::Int32
1771-
flag::UInt8
1772-
function NewInstruction(@nospecialize(stmt), @nospecialize(type), @nospecialize(info::CallInfo),
1773-
line::Int32, flag::UInt8)
1774-
return new(stmt, type, info, line, flag)
1775-
end
1776-
end
1777-
@nospecialize
1778-
function NewInstruction(newinst::NewInstruction;
1779-
stmt=newinst.stmt,
1780-
type=newinst.type,
1781-
info::CallInfo=newinst.info,
1782-
line::Int32=newinst.line,
1783-
flag::UInt8=newinst.flag)
1784-
return NewInstruction(stmt, type, info, line, flag)
1785-
end
1786-
@specialize
1787-
let src = code_typed1((NewInstruction,Any,Any,CallInfo)) do newinst, stmt, type, info
1788-
NewInstruction(newinst; stmt, type, info)
1789-
end
1790-
@test count(issplatnew, src.code) == 0
1791-
@test count(iscall((src,NamedTuple)), src.code) == 0
1792-
@test count(isnew, src.code) == 1
1793-
end
17941752

17951753
# Test that inlining can still use nothrow information from concrete-eval
17961754
# even if the result itself is too big to be inlined, and nothrow is not

test/compiler/irutils.jl

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ get_code(args...; kwargs...) = code_typed1(args...; kwargs...).code
88

99
# check if `x` is a statement with a given `head`
1010
isnew(@nospecialize x) = isexpr(x, :new)
11-
issplatnew(@nospecialize x) = isexpr(x, :splatnew)
1211
isreturn(@nospecialize x) = isa(x, ReturnNode)
1312

1413
# check if `x` is a dynamic call of a given function

0 commit comments

Comments
 (0)