Skip to content

Commit

Permalink
inference: fix return_type_tfunc modeling of concrete functions (#51042)
Browse files Browse the repository at this point in the history 
The `aft` parameter is a value already, so we should be checking it in
the value domain, not the type domain like `tt`. That check happens to
already be done (somewhat unnecessarily) earlier in the function.

Fixes #40606

---------

Co-authored-by: Shuhei Kadowaki <[email protected]>
  • Loading branch information
@vtjnash @aviatesk
vtjnash and aviatesk authored Aug 27, 2023
1 parent ca1a54a commit f24a93a
Showing 1 changed file with 7 additions and 6 deletions.
13 changes: 7 additions & 6 deletions base/compiler/tfuncs.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2638,13 +2638,15 @@ function return_type_tfunc(interp::AbstractInterpreter, argtypes::Vector{Any}, s

if length(argtypes) == 3
aft = widenslotwrapper(argtypes[2])
if !isa(aft, Const) && !(isType(aft) && !has_free_typevars(aft)) &&
!(isconcretetype(aft) && !(aft <: Builtin))
return UNKNOWN
end
argtypes_vec = Any[aft, af_argtype.parameters...]
else
argtypes_vec = Any[af_argtype.parameters...]
isempty(argtypes_vec) && push!(argtypes_vec, Union{})
aft = argtypes_vec[1]
end
if !(isa(aft, Const) || (isType(aft) && !has_free_typevars(aft)) ||
(isconcretetype(aft) && !(aft <: Builtin) && !iskindtype(aft)))
return UNKNOWN
end

if contains_is(argtypes_vec, Union{})
Expand Down Expand Up @@ -2677,8 +2679,7 @@ function return_type_tfunc(interp::AbstractInterpreter, argtypes::Vector{Any}, s
# in two ways: both as being a subtype of this, and
# because of LimitedAccuracy causes
return CallMeta(Type{<:rt}, EFFECTS_TOTAL, info)
elseif (isa(tt, Const) || isconstType(tt)) &&
(isa(aft, Const) || isconstType(aft))
elseif isa(tt, Const) || isconstType(tt)
# input arguments were known for certain
# XXX: this doesn't imply we know anything about rt
return CallMeta(Const(rt), EFFECTS_TOTAL, info)
Expand Down

5 comments on commit f24a93a

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

@vtjnash
Copy link
Member Author

Choose a reason for hiding this comment

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

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

@vtjnash
Copy link
Member Author

Choose a reason for hiding this comment

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

This was maybe from the SparseArray bump, but a bit unexpected:

["array", "cat", ("hcat", 5)]	1.07 (5%) ❌	1.01 (1%) ❌
["array", "cat", ("vcat", 5)]	1.06 (5%) ❌	1.01 (1%) ❌
["sparse", "index", ("spvec", "logical", 10000)]	2.63 (30%) ❌	1.00 (1%)
["sparse", "index", ("spvec", "logical", 100000)]	3.11 (30%) ❌	1.00 (1%)

This seems all unexpected:

["collection", "deletion", ("IdDict", "Int", "filter")]	1.05 (25%)	1.08 (1%) ❌
["collection", "initialization", ("IdDict", "Int", "iterator")]	1.08 (25%)	1.19 (1%) ❌
["collection", "initialization", ("IdDict", "Int", "loop")]	1.05 (25%)	1.19 (1%) ❌
["collection", "initialization", ("IdDict", "Int", "loop", "sizehint!")]	1.05 (25%)	1.24 (1%) ❌
["collection", "optimizations", ("IdDict", "abstract", "UInt16")]	1.09 (25%)	1.35 (1%) ❌
["collection", "optimizations", ("IdDict", "concrete", "UInt16")]	1.08 (25%)	1.35 (1%) ❌
["collection", "optimizations", ("Vector", "concrete", "Nothing")]	1.49 (25%) ❌	1.00 (1%)
["collection", "queries & updates", ("IdDict", "Int", "pop!", "specified")]	1.05 (25%)	1.25 (1%) ❌
["collection", "queries & updates", ("IdDict", "Int", "push!", "overwrite")]	1.10 (25%)	1.50 (1%) ❌

This was expected (and intended):

["find", "findall", ("> q0.5", "Vector{Bool}")]	0.32 (5%) ✅	2.09 (1%) ❌
["find", "findall", ("> q0.8", "Vector{Bool}")]	1.53 (5%) ❌	47.67 (1%) ❌
["find", "findall", ("> q0.95", "Vector{Bool}")]	1.50 (5%) ❌	47.67 (1%) ❌
["find", "findall", ("> q0.99", "Vector{Bool}")]	1.50 (5%) ❌	47.67 (1%) ❌
["find", "findall", ("ispos", "Vector{Bool}")]	1.05 (5%) ❌	2.10 (1%) ❌

This is cool:

["inference", "abstract interpretation", "many_global_refs"]	0.11 (5%) ✅	1.00 (1%)
["inference", "allinference", "many_global_refs"]	0.21 (5%) ✅	1.00 (1%)

Please sign in to comment.