Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

follow up #44448, correct findall/findsup for OverlayMethodTable #44511

Merged
merged 1 commit into from
Mar 9, 2022
Merged

follow up #44448, correct findall/findsup for OverlayMethodTable #44511

merged 1 commit into from
Mar 9, 2022

Conversation

aviatesk
Copy link
Member

@aviatesk aviatesk commented Mar 8, 2022

Address the review comments by @vtjnash on #44448:

@aviatesk aviatesk requested a review from vtjnash March 8, 2022 08:47
aviatesk added a commit that referenced this pull request Mar 8, 2022
@aviatesk
AbstractInterpreter: enable partial pure/concrete eval for external…
81c0cd1 
… `AbstractInterpreter` with overlayed method table

Built on top of #44511, and solves <JuliaGPU/GPUCompiler.jl#309>.
This commit allows external `AbstractInterpreter` to use pure/concrete
evals even if it uses an overlayed method table. More specifically, such
`AbstractInterpreter` can use pure/concrete evals as far as any matching
methods in question doesn't come from the overlayed method table:
```julia
@test Base.return_types((), MTOverlayInterp()) do
    isbitstype(Int) ? nothing : missing
end == Any[Nothing]
Base.@assume_effects :terminates_globally function issue41694(x)
    res = 1
    1 < x < 20 || throw("bad")
    while x > 1
        res *= x
        x -= 1
    end
    return res
end
@test Base.return_types((), MTOverlayInterp()) do
    issue41694(3) == 6 ? nothing : missing
end == Any[Nothing]
```
@aviatesk aviatesk mentioned this pull request Mar 8, 2022
Merged
@aviatesk aviatesk added the backport 1.8 Change should be backported to release-1.8 label Mar 8, 2022
vtjnash
vtjnash reviewed Mar 8, 2022
View reviewed changes
base/compiler/abstractinterpretation.jl Outdated
result === nothing && return CallMeta(Any, false)
match, valid_worlds = result
method = match.method
match, valid_worlds = findsup(types, method_table(interp))
update_valid_age!(sv, valid_worlds)
Copy link
Member

Choose a reason for hiding this comment

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

If you return Any, there is no reason to need to update the valid worlds, since you did not use any information from the look up.

Copy link
Member Author

Choose a reason for hiding this comment

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

I wondered if this allows us to invalidate unsuccessful inference trial when a new method comes to be available, e.g.

ulia> foo(::Int) = :Int
foo (generic function with 1 method)

julia> CC.findsup(Tuple{typeof(foo),Integer}, CC.method_table(CC.NativeInterpreter()))
(nothing, Core.Compiler.WorldRange(0x0000000000000000, 0xffffffffffffffff))

julia> callfoo(n) = Base.@invoke foo(n::Integer)
callfoo (generic function with 1 method)

julia> callfoo2(n) = callfoo(n)
callfoo2 (generic function with 1 method)

julia> @code_typed callfoo2(42)
CodeInfo(
1 ─ %1 = Core.invoke(Main.foo, Tuple{Integer}, n)::Any
└──      return %1
) => Any

julia> foo(::Integer) = :Integer
foo (generic function with 2 methods)

julia> CC.findsup(Tuple{typeof(foo),Integer}, CC.method_table(CC.NativeInterpreter()))
(Core.MethodMatch(Tuple{typeof(foo), Integer}, svec(), foo(::Integer) in Main at REPL[6]:1, true), Core.Compiler.WorldRange(0x0000000000007e7e, 0xffffffffffffffff))

julia> @code_typed callfoo2(42)
CodeInfo(
1 ─ %1 = Core.invoke(Main.foo, Tuple{Integer}, n)::Any
└──      return %1
) => Any

But as indicated at these lines, it seems like we don't pick up a valid minimum world age when the match failed, so this update_valid_age! would be really ineffective in this kind of case.

julia> CC.findsup(Tuple{typeof(foo),Integer}, CC.method_table(CC.NativeInterpreter()))
(nothing, Core.Compiler.WorldRange(0x0000000000000000, 0xffffffffffffffff))

vtjnash
vtjnash reviewed Mar 8, 2022
View reviewed changes
Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

LGTM

@aviatesk
follow up #44448, correct findall/findsup for OverlayMethodTable
e6311dd 
- respect world range of failed lookup into `OverlayMethodTable`:
  <#44448 (comment)>
- fix calculation of merged valid world range:
  <#44448 (comment)>
- make `findsup` return valid `WorldRange` unconditionally, and enable
  more strict world validation within `abstract_invoke`:
  <#44448 (comment)>
- fix the default `limit::Int` value of `findall`
aviatesk added a commit that referenced this pull request Mar 9, 2022
@aviatesk
AbstractInterpreter: enable partial pure/concrete eval for external…
6635c1b 
… `AbstractInterpreter` with overlayed method table

Built on top of #44511, and solves <JuliaGPU/GPUCompiler.jl#309>.
This commit allows external `AbstractInterpreter` to use pure/concrete
evals even if it uses an overlayed method table. More specifically, such
`AbstractInterpreter` can use pure/concrete evals as far as any matching
methods in question doesn't come from the overlayed method table:
```julia
@test Base.return_types((), MTOverlayInterp()) do
    isbitstype(Int) ? nothing : missing
end == Any[Nothing]
Base.@assume_effects :terminates_globally function issue41694(x)
    res = 1
    1 < x < 20 || throw("bad")
    while x > 1
        res *= x
        x -= 1
    end
    return res
end
@test Base.return_types((), MTOverlayInterp()) do
    issue41694(3) == 6 ? nothing : missing
end == Any[Nothing]
```
@aviatesk
Copy link
Member Author

aviatesk commented Mar 9, 2022

Failures on macos64 seems unrelated.

@aviatesk aviatesk merged commit 6e8804b into master Mar 9, 2022
@aviatesk aviatesk deleted the avi/follow44448 branch March 9, 2022 07:34
aviatesk added a commit that referenced this pull request Mar 9, 2022
@aviatesk
follow up #44448, correct findall/findsup for OverlayMethodTable (
0a4a850 
#44511)

- respect world range of failed lookup into `OverlayMethodTable`:
  <#44448 (comment)>
- fix calculation of merged valid world range:
  <#44448 (comment)>
- make `findsup` return valid `WorldRange` unconditionally, and enable
  more strict world validation within `abstract_invoke`:
  <#44448 (comment)>
- fix the default `limit::Int` value of `findall`
@aviatesk aviatesk mentioned this pull request Mar 9, 2022
Merged
47 tasks
aviatesk added a commit that referenced this pull request Mar 9, 2022
@aviatesk
AbstractInterpreter: enable partial pure/concrete eval for external…
7f1e461 
… `AbstractInterpreter` with overlayed method table

Built on top of #44511, and solves <JuliaGPU/GPUCompiler.jl#309>.
This commit allows external `AbstractInterpreter` to use pure/concrete
evals even if it uses an overlayed method table. More specifically, such
`AbstractInterpreter` can use pure/concrete evals as far as any matching
methods in question doesn't come from the overlayed method table:
```julia
@test Base.return_types((), MTOverlayInterp()) do
    isbitstype(Int) ? nothing : missing
end == Any[Nothing]
Base.@assume_effects :terminates_globally function issue41694(x)
    res = 1
    1 < x < 20 || throw("bad")
    while x > 1
        res *= x
        x -= 1
    end
    return res
end
@test Base.return_types((), MTOverlayInterp()) do
    issue41694(3) == 6 ? nothing : missing
end == Any[Nothing]
```
aviatesk added a commit that referenced this pull request Mar 11, 2022
@aviatesk
AbstractInterpreter: enable partial pure/concrete eval for external…
383c4e0 
… `AbstractInterpreter` with overlayed method table

Built on top of #44511, and solves <JuliaGPU/GPUCompiler.jl#309>.
This commit allows external `AbstractInterpreter` to use pure/concrete
evals even if it uses an overlayed method table. More specifically, such
`AbstractInterpreter` can use pure/concrete evals as far as any matching
methods in question doesn't come from the overlayed method table:
```julia
@test Base.return_types((), MTOverlayInterp()) do
    isbitstype(Int) ? nothing : missing
end == Any[Nothing]
Base.@assume_effects :terminates_globally function issue41694(x)
    res = 1
    1 < x < 20 || throw("bad")
    while x > 1
        res *= x
        x -= 1
    end
    return res
end
@test Base.return_types((), MTOverlayInterp()) do
    issue41694(3) == 6 ? nothing : missing
end == Any[Nothing]
```
aviatesk added a commit that referenced this pull request Mar 14, 2022
@aviatesk
AbstractInterpreter: enable partial pure/concrete eval for external…
86bcb86 
… `AbstractInterpreter` with overlayed method table

Built on top of #44511, and solves <JuliaGPU/GPUCompiler.jl#309>.
This commit allows external `AbstractInterpreter` to use pure/concrete
evals even if it uses an overlayed method table. More specifically, such
`AbstractInterpreter` can use pure/concrete evals as far as any matching
methods in question doesn't come from the overlayed method table:
```julia
@test Base.return_types((), MTOverlayInterp()) do
    isbitstype(Int) ? nothing : missing
end == Any[Nothing]
Base.@assume_effects :terminates_globally function issue41694(x)
    res = 1
    1 < x < 20 || throw("bad")
    while x > 1
        res *= x
        x -= 1
    end
    return res
end
@test Base.return_types((), MTOverlayInterp()) do
    issue41694(3) == 6 ? nothing : missing
end == Any[Nothing]
```
aviatesk added a commit that referenced this pull request Mar 14, 2022
@aviatesk
AbstractInterpreter: enable selective pure/concrete eval for extern…
7ae3491 
…al `AbstractInterpreter` with overlayed method table

Built on top of #44511 and #44561, and solves <JuliaGPU/GPUCompiler.jl#309>.
This commit allows external `AbstractInterpreter` to selectively use
pure/concrete evals even if it uses an overlayed method table.
More specifically, such `AbstractInterpreter` can use pure/concrete evals
as far as any callees used in a call in question doesn't come from the
overlayed method table:
```julia
@test Base.return_types((), MTOverlayInterp()) do
    isbitstype(Int) ? nothing : missing
end == Any[Nothing]
Base.@assume_effects :terminates_globally function issue41694(x)
    res = 1
    1 < x < 20 || throw("bad")
    while x > 1
        res *= x
        x -= 1
    end
    return res
end
@test Base.return_types((), MTOverlayInterp()) do
    issue41694(3) == 6 ? nothing : missing
end == Any[Nothing]
```

In order to check if a call is tainted by any overlayed call, our effect
system now additionally tracks `overlayed::Bool` property. This effect
property is required to prevents concrete-eval in the following kind of situation:
```julia
strangesin(x) = sin(x)
@overlay OverlayedMT strangesin(x::Float64) = iszero(x) ? nothing : cos(x)
Base.@assume_effects :total totalcall(f, args...) = f(args...)
@test Base.return_types(; interp=MTOverlayInterp()) do
    # we need to disable partial pure/concrete evaluation when tainted by any overlayed call
    if totalcall(strangesin, 1.0) == cos(1.0)
        return nothing
    else
        return missing
    end
end |> only === Nothing
```
aviatesk added a commit that referenced this pull request Mar 14, 2022
@aviatesk
AbstractInterpreter: enable selective pure/concrete eval for extern…
5207c46 
…al `AbstractInterpreter` with overlayed method table

Built on top of #44511 and #44561, and solves <JuliaGPU/GPUCompiler.jl#309>.
This commit allows external `AbstractInterpreter` to selectively use
pure/concrete evals even if it uses an overlayed method table.
More specifically, such `AbstractInterpreter` can use pure/concrete evals
as far as any callees used in a call in question doesn't come from the
overlayed method table:
```julia
@test Base.return_types((), MTOverlayInterp()) do
    isbitstype(Int) ? nothing : missing
end == Any[Nothing]
Base.@assume_effects :terminates_globally function issue41694(x)
    res = 1
    1 < x < 20 || throw("bad")
    while x > 1
        res *= x
        x -= 1
    end
    return res
end
@test Base.return_types((), MTOverlayInterp()) do
    issue41694(3) == 6 ? nothing : missing
end == Any[Nothing]
```

In order to check if a call is tainted by any overlayed call, our effect
system now additionally tracks `overlayed::Bool` property. This effect
property is required to prevents concrete-eval in the following kind of situation:
```julia
strangesin(x) = sin(x)
@overlay OverlayedMT strangesin(x::Float64) = iszero(x) ? nothing : cos(x)
Base.@assume_effects :total totalcall(f, args...) = f(args...)
@test Base.return_types(; interp=MTOverlayInterp()) do
    # we need to disable partial pure/concrete evaluation when tainted by any overlayed call
    if totalcall(strangesin, 1.0) == cos(1.0)
        return nothing
    else
        return missing
    end
end |> only === Nothing
```
aviatesk added a commit that referenced this pull request Mar 14, 2022
@aviatesk
AbstractInterpreter: enable selective pure/concrete eval for extern…
44ebba2 
…al `AbstractInterpreter` with overlayed method table

Built on top of #44511 and #44561, and solves <JuliaGPU/GPUCompiler.jl#309>.
This commit allows external `AbstractInterpreter` to selectively use
pure/concrete evals even if it uses an overlayed method table.
More specifically, such `AbstractInterpreter` can use pure/concrete evals
as far as any callees used in a call in question doesn't come from the
overlayed method table:
```julia
@test Base.return_types((), MTOverlayInterp()) do
    isbitstype(Int) ? nothing : missing
end == Any[Nothing]
Base.@assume_effects :terminates_globally function issue41694(x)
    res = 1
    1 < x < 20 || throw("bad")
    while x > 1
        res *= x
        x -= 1
    end
    return res
end
@test Base.return_types((), MTOverlayInterp()) do
    issue41694(3) == 6 ? nothing : missing
end == Any[Nothing]
```

In order to check if a call is tainted by any overlayed call, our effect
system now additionally tracks `overlayed::Bool` property. This effect
property is required to prevents concrete-eval in the following kind of situation:
```julia
strangesin(x) = sin(x)
@overlay OverlayedMT strangesin(x::Float64) = iszero(x) ? nothing : cos(x)
Base.@assume_effects :total totalcall(f, args...) = f(args...)
@test Base.return_types(; interp=MTOverlayInterp()) do
    # we need to disable partial pure/concrete evaluation when tainted by any overlayed call
    if totalcall(strangesin, 1.0) == cos(1.0)
        return nothing
    else
        return missing
    end
end |> only === Nothing
```
aviatesk added a commit that referenced this pull request Mar 14, 2022
@aviatesk
AbstractInterpreter: enable selective pure/concrete eval for extern…
53fed22 
…al `AbstractInterpreter` with overlayed method table

Built on top of #44511 and #44561, and solves <JuliaGPU/GPUCompiler.jl#309>.
This commit allows external `AbstractInterpreter` to selectively use
pure/concrete evals even if it uses an overlayed method table.
More specifically, such `AbstractInterpreter` can use pure/concrete evals
as far as any callees used in a call in question doesn't come from the
overlayed method table:
```julia
@test Base.return_types((), MTOverlayInterp()) do
    isbitstype(Int) ? nothing : missing
end == Any[Nothing]
Base.@assume_effects :terminates_globally function issue41694(x)
    res = 1
    1 < x < 20 || throw("bad")
    while x > 1
        res *= x
        x -= 1
    end
    return res
end
@test Base.return_types((), MTOverlayInterp()) do
    issue41694(3) == 6 ? nothing : missing
end == Any[Nothing]
```

In order to check if a call is tainted by any overlayed call, our effect
system now additionally tracks `overlayed::Bool` property. This effect
property is required to prevents concrete-eval in the following kind of situation:
```julia
strangesin(x) = sin(x)
@overlay OverlayedMT strangesin(x::Float64) = iszero(x) ? nothing : cos(x)
Base.@assume_effects :total totalcall(f, args...) = f(args...)
@test Base.return_types(; interp=MTOverlayInterp()) do
    # we need to disable partial pure/concrete evaluation when tainted by any overlayed call
    if totalcall(strangesin, 1.0) == cos(1.0)
        return nothing
    else
        return missing
    end
end |> only === Nothing
```
aviatesk added a commit that referenced this pull request Mar 14, 2022
@aviatesk
AbstractInterpreter: enable selective pure/concrete eval for extern…
013834a 
…al `AbstractInterpreter` with overlayed method table

Built on top of #44511 and #44561, and solves <JuliaGPU/GPUCompiler.jl#309>.
This commit allows external `AbstractInterpreter` to selectively use
pure/concrete evals even if it uses an overlayed method table.
More specifically, such `AbstractInterpreter` can use pure/concrete evals
as far as any callees used in a call in question doesn't come from the
overlayed method table:
```julia
@test Base.return_types((), MTOverlayInterp()) do
    isbitstype(Int) ? nothing : missing
end == Any[Nothing]
Base.@assume_effects :terminates_globally function issue41694(x)
    res = 1
    1 < x < 20 || throw("bad")
    while x > 1
        res *= x
        x -= 1
    end
    return res
end
@test Base.return_types((), MTOverlayInterp()) do
    issue41694(3) == 6 ? nothing : missing
end == Any[Nothing]
```

In order to check if a call is tainted by any overlayed call, our effect
system now additionally tracks `overlayed::Bool` property. This effect
property is required to prevents concrete-eval in the following kind of situation:
```julia
strangesin(x) = sin(x)
@overlay OverlayedMT strangesin(x::Float64) = iszero(x) ? nothing : cos(x)
Base.@assume_effects :total totalcall(f, args...) = f(args...)
@test Base.return_types(; interp=MTOverlayInterp()) do
    # we need to disable partial pure/concrete evaluation when tainted by any overlayed call
    if totalcall(strangesin, 1.0) == cos(1.0)
        return nothing
    else
        return missing
    end
end |> only === Nothing
```
aviatesk added a commit that referenced this pull request Mar 14, 2022
@aviatesk
AbstractInterpreter: enable selective pure/concrete eval for extern…
f840163 
…al `AbstractInterpreter` with overlayed method table

Built on top of #44511 and #44561, and solves <JuliaGPU/GPUCompiler.jl#309>.
This commit allows external `AbstractInterpreter` to selectively use
pure/concrete evals even if it uses an overlayed method table.
More specifically, such `AbstractInterpreter` can use pure/concrete evals
as far as any callees used in a call in question doesn't come from the
overlayed method table:
```julia
@test Base.return_types((), MTOverlayInterp()) do
    isbitstype(Int) ? nothing : missing
end == Any[Nothing]
Base.@assume_effects :terminates_globally function issue41694(x)
    res = 1
    1 < x < 20 || throw("bad")
    while x > 1
        res *= x
        x -= 1
    end
    return res
end
@test Base.return_types((), MTOverlayInterp()) do
    issue41694(3) == 6 ? nothing : missing
end == Any[Nothing]
```

In order to check if a call is tainted by any overlayed call, our effect
system now additionally tracks `overlayed::Bool` property. This effect
property is required to prevents concrete-eval in the following kind of situation:
```julia
strangesin(x) = sin(x)
@overlay OverlayedMT strangesin(x::Float64) = iszero(x) ? nothing : cos(x)
Base.@assume_effects :total totalcall(f, args...) = f(args...)
@test Base.return_types(; interp=MTOverlayInterp()) do
    # we need to disable partial pure/concrete evaluation when tainted by any overlayed call
    if totalcall(strangesin, 1.0) == cos(1.0)
        return nothing
    else
        return missing
    end
end |> only === Nothing
```
aviatesk added a commit that referenced this pull request Mar 15, 2022
@aviatesk
AbstractInterpreter: enable selective pure/concrete eval for extern…
dba4df7 
…al `AbstractInterpreter` with overlayed method table

Built on top of #44511 and #44561, and solves <JuliaGPU/GPUCompiler.jl#309>.
This commit allows external `AbstractInterpreter` to selectively use
pure/concrete evals even if it uses an overlayed method table.
More specifically, such `AbstractInterpreter` can use pure/concrete evals
as far as any callees used in a call in question doesn't come from the
overlayed method table:
```julia
@test Base.return_types((), MTOverlayInterp()) do
    isbitstype(Int) ? nothing : missing
end == Any[Nothing]
Base.@assume_effects :terminates_globally function issue41694(x)
    res = 1
    1 < x < 20 || throw("bad")
    while x > 1
        res *= x
        x -= 1
    end
    return res
end
@test Base.return_types((), MTOverlayInterp()) do
    issue41694(3) == 6 ? nothing : missing
end == Any[Nothing]
```

In order to check if a call is tainted by any overlayed call, our effect
system now additionally tracks `overlayed::Bool` property. This effect
property is required to prevents concrete-eval in the following kind of situation:
```julia
strangesin(x) = sin(x)
@overlay OverlayedMT strangesin(x::Float64) = iszero(x) ? nothing : cos(x)
Base.@assume_effects :total totalcall(f, args...) = f(args...)
@test Base.return_types(; interp=MTOverlayInterp()) do
    # we need to disable partial pure/concrete evaluation when tainted by any overlayed call
    if totalcall(strangesin, 1.0) == cos(1.0)
        return nothing
    else
        return missing
    end
end |> only === Nothing
```
aviatesk added a commit that referenced this pull request Mar 15, 2022
@aviatesk
AbstractInterpreter: enable selective pure/concrete eval for extern…
c0c14b6 
…al `AbstractInterpreter` with overlayed method table

Built on top of #44511 and #44561, and solves <JuliaGPU/GPUCompiler.jl#309>.
This commit allows external `AbstractInterpreter` to selectively use
pure/concrete evals even if it uses an overlayed method table.
More specifically, such `AbstractInterpreter` can use pure/concrete evals
as far as any callees used in a call in question doesn't come from the
overlayed method table:
```julia
@test Base.return_types((), MTOverlayInterp()) do
    isbitstype(Int) ? nothing : missing
end == Any[Nothing]
Base.@assume_effects :terminates_globally function issue41694(x)
    res = 1
    1 < x < 20 || throw("bad")
    while x > 1
        res *= x
        x -= 1
    end
    return res
end
@test Base.return_types((), MTOverlayInterp()) do
    issue41694(3) == 6 ? nothing : missing
end == Any[Nothing]
```

In order to check if a call is tainted by any overlayed call, our effect
system now additionally tracks `overlayed::Bool` property. This effect
property is required to prevents concrete-eval in the following kind of situation:
```julia
strangesin(x) = sin(x)
@overlay OverlayedMT strangesin(x::Float64) = iszero(x) ? nothing : cos(x)
Base.@assume_effects :total totalcall(f, args...) = f(args...)
@test Base.return_types(; interp=MTOverlayInterp()) do
    # we need to disable partial pure/concrete evaluation when tainted by any overlayed call
    if totalcall(strangesin, 1.0) == cos(1.0)
        return nothing
    else
        return missing
    end
end |> only === Nothing
```
@KristofferC KristofferC removed the backport 1.8 Change should be backported to release-1.8 label Mar 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vtjnash vtjnash vtjnash left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@aviatesk @vtjnash @KristofferC