Skip to content

Commit

Permalink
fix #29036, poor inference of val,i = iterate(x,i)
Browse files Browse the repository at this point in the history
In this case, the result of `iterate` has not been checked for
`nothing`, so we try to call `indexed_iterate` (for destructuring
assignment) on a Union of Nothing and the tuple returned by
`iterate`. That has two method matches, and so was excluded from
constant propagation. This commit fixes that by generalizing the
constant prop heuristic from requiring one method match to
requiring one non-Bottom method match.

This issue caused a large slowdown in DelimitedFiles, where
the inner loop consists of

```
        while idx <= slen
            val,idx = iterate(dbuff, idx)
```
  • Loading branch information
JeffBezanson committed Sep 7, 2018
1 parent bda5dd8 commit 957b9c0
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 9 deletions.
27 changes: 19 additions & 8 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,15 @@ function abstract_call_gf_by_type(@nospecialize(f), argtypes::Vector{Any}, @nosp
rettype = Bottom
edgecycle = false
edges = Any[]
nonbot = 0 # the index of the only non-Bottom inference result if > 0
seen = 0 # number of signatures actually inferred
for i in 1:napplicable
match = applicable[i]::SimpleVector
method = match[3]::Method
sig = match[1]
sigtuple = unwrap_unionall(sig)::DataType
splitunions = false
this_rt = Bottom
# TODO: splitunions = 1 < countunionsplit(sigtuple.parameters) * napplicable <= sv.params.MAX_UNION_SPLITTING
# currently this triggers a bug in inference recursion detection
if splitunions
Expand All @@ -71,24 +74,32 @@ function abstract_call_gf_by_type(@nospecialize(f), argtypes::Vector{Any}, @nosp
push!(edges, edge)
end
edgecycle |= edgecycle1::Bool
rettype = tmerge(rettype, rt)
rettype === Any && break
this_rt = tmerge(this_rt, rt)
this_rt === Any && break
end
rettype === Any && break
else
rt, edgecycle, edge = abstract_call_method(method, sig, match[2]::SimpleVector, sv)
this_rt, edgecycle, edge = abstract_call_method(method, sig, match[2]::SimpleVector, sv)
if edge !== nothing
push!(edges, edge)
end
rettype = tmerge(rettype, rt)
rettype === Any && break
end
if this_rt !== Bottom
if nonbot === 0
nonbot = i
else
nonbot = -1
end
end
seen += 1
rettype = tmerge(rettype, this_rt)
rettype === Any && break
end
if napplicable == 1 && !edgecycle && isa(rettype, Type) && sv.params.ipo_constant_propagation
# try constant propagation if only 1 method is inferred to non-Bottom
if nonbot > 0 && seen == napplicable && !edgecycle && isa(rettype, Type) && sv.params.ipo_constant_propagation
# if there's a possibility we could constant-propagate a better result
# (hopefully without doing too much work), try to do that now
# TODO: it feels like this could be better integrated into abstract_call_method / typeinf_edge
const_rettype = abstract_call_method_with_const_args(f, argtypes, applicable[1]::SimpleVector, sv)
const_rettype = abstract_call_method_with_const_args(f, argtypes, applicable[nonbot]::SimpleVector, sv)
if const_rettype rettype
# use the better result, if it's a refinement of rettype
rettype = const_rettype
Expand Down
2 changes: 1 addition & 1 deletion base/tuple.jl
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ end

# this allows partial evaluation of bounded sequences of next() calls on tuples,
# while reducing to plain next() for arbitrary iterables.
indexed_iterate(t::Tuple, i::Int, state=1) = (@_inline_meta; (t[i], i+1))
indexed_iterate(t::Tuple, i::Int, state=1) = (@_inline_meta; (getfield(t, i), i+1))
indexed_iterate(a::Array, i::Int, state=1) = (@_inline_meta; (a[i], i+1))
function indexed_iterate(I, i)
x = iterate(I)
Expand Down
7 changes: 7 additions & 0 deletions test/compiler/compiler.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1990,3 +1990,10 @@ struct VoxelIndices{T <: Integer}
end
f28641(x::VoxelIndices, f) = getfield(x, f)
@test Base.return_types(f28641, (Any,Symbol)) == Any[Tuple]

# issue #29036
function f29036(s, i)
val, i = iterate(s, i)
val
end
@test Base.return_types(f29036, (String, Int)) == Any[Char]

0 comments on commit 957b9c0

Please sign in to comment.