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

fix partially_inline! with isdefined #40628

Merged
merged 3 commits into from
Jul 1, 2021
Merged

fix partially_inline! with isdefined #40628

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3d67fbc
fix `partially_inline!` with `isdefined`
simeonschaub Apr 27, 2021
e397664
allow undefined sparam replacements
simeonschaub Apr 30, 2021
7a53ad8
Merge remote-tracking branch 'origin/master' into sds/inline_isdefined
simeonschaub Jul 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions base/meta.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,27 @@ function _partially_inline!(@nospecialize(x), slot_replacements::Vector{Any},
x.args[2] += statement_offset
elseif head === :enter
x.args[1] += statement_offset
elseif head === :isdefined
arg = x.args[1]
# inlining a QuoteNode or literal into `Expr(:isdefined, x)` is invalid, replace with true
if isa(arg, Core.SlotNumber)
id = arg.id
if 1 <= id <= length(slot_replacements)
replacement = slot_replacements[id]
if isa(replacement, Union{Core.SlotNumber, GlobalRef, Symbol})
return Expr(:isdefined, replacement)
else
@assert !isa(replacement, Expr)
return true
end
end
return Expr(:isdefined, Core.SlotNumber(id + slot_offset))
elseif isexpr(arg, :static_parameter)
Copy link
Member

Choose a reason for hiding this comment

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

Not sure if applicable

julia> f() where T = @isdefined(T)
f (generic function with 1 method)

julia> f()
false
julia> @code_typed f()
CodeInfo(
1 ─ %1 = $(Expr(:isdefined, :($(Expr(:static_parameter, 1)))))::Bool
└──      return %1
) => Bool

So you can have a not defined static_parameter

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good point, but for partially_inline! to work, you would still need to provide a value for the static parameter. Perhaps we should allow the sparam replacements to have undefined entries and only replace with true if the replacement is defined?

Copy link
Member Author

Choose a reason for hiding this comment

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

Does this look good to you?

Copy link
Member

Choose a reason for hiding this comment

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

We typically use TypeVars to indicate an unknown static parameter, but this function doesn't seem to support that (yet) so we can leave that for now, and this looks good.

return true
else
@assert isa(arg, Union{GlobalRef, Symbol})
return x
end
elseif !is_meta_expr_head(head)
partially_inline!(x.args, slot_replacements, type_signature, static_param_values,
slot_offset, statement_offset, boundscheck)
Expand Down
28 changes: 24 additions & 4 deletions test/meta.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,27 @@ ci = code_lowered(f, Tuple{Int})[1]

g(::Val{x}) where {x} = x ? 1 : 0
ci = code_lowered(g, Tuple{Val{true}})[1]
@test Meta.partially_inline!(ci.code, [], Tuple{typeof(g),Val{true}}, Any[Val{true}], 0, 0, :propagate)[1] ==
Core.GotoIfNot(QuoteNode(Val{true}), 3)
@test Meta.partially_inline!(ci.code, [], Tuple{typeof(g),Val{true}}, Any[Val{true}], 0, 2, :propagate)[1] ==
Core.GotoIfNot(QuoteNode(Val{true}), 5)
@test Meta.partially_inline!(ci.code, [], Tuple{typeof(g),Val{true}}, Any[true], 0, 0, :propagate)[1] ==
Core.GotoIfNot(QuoteNode(true), 3)
@test Meta.partially_inline!(ci.code, [], Tuple{typeof(g),Val{true}}, Any[true], 0, 2, :propagate)[1] ==
Core.GotoIfNot(QuoteNode(true), 5)

@testset "inlining with isdefined" begin
isdefined_slot(x) = @isdefined(x)
ci = code_lowered(isdefined_slot, Tuple{Int})[1]
@test Meta.partially_inline!(ci.code, [], Tuple{typeof(isdefined_slot), Int},
Any[Int], 0, 0, :propagate)[1] == Expr(:isdefined, Core.SlotNumber(2))
@test Meta.partially_inline!(ci.code, [isdefined_slot, 1], Tuple{typeof(isdefined_slot), Int},
Any[Int], 0, 0, :propagate)[1] == true

isdefined_sparam(::T) where {T} = @isdefined(T)
ci = code_lowered(isdefined_sparam, Tuple{Int})[1]
@test Meta.partially_inline!(ci.code, [], Tuple{typeof(isdefined_sparam), Int},
Any[Int], 0, 0, :propagate)[1] == true

@eval isdefined_globalref(x) = $(Expr(:isdefined, GlobalRef(Base, :foo)))
ci = code_lowered(isdefined_globalref, Tuple{Int})[1]
@test Meta.partially_inline!(ci.code, Any[isdefined_globalref, 1], Tuple{typeof(isdefined_globalref), Int},
Any[Int], 0, 0, :propagate)[1] == Expr(:isdefined, GlobalRef(Base, :foo))

end