Skip to content

Commit

Permalink
propagate method metadata to keyword sorter methods (#45041)
Browse files Browse the repository at this point in the history
Co-authored-by: Shuhei Kadowaki <[email protected]>
  • Loading branch information
JeffBezanson and aviatesk authored Jul 13, 2022
1 parent c1750f2 commit 9edac22
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 3 deletions.
5 changes: 2 additions & 3 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -827,8 +827,7 @@ function const_prop_enabled(interp::AbstractInterpreter, sv::InferenceState, mat
add_remark!(interp, sv, "[constprop] Disabled by parameter")
return false
end
method = match.method
if method.constprop == 0x02
if is_no_constprop(match.method)
add_remark!(interp, sv, "[constprop] Disabled by method parameter")
return false
end
Expand Down Expand Up @@ -1028,7 +1027,7 @@ function is_all_overridden((; fargs, argtypes)::ArgInfo, sv::InferenceState)
end

function force_const_prop(interp::AbstractInterpreter, @nospecialize(f), method::Method)
return method.constprop == 0x01 ||
return is_aggressive_constprop(method) ||
InferenceParams(interp).aggressive_constant_propagation ||
istopfunction(f, :getproperty) ||
istopfunction(f, :setproperty!)
Expand Down
14 changes: 14 additions & 0 deletions base/compiler/utilities.jl
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,20 @@ function specialize_method(match::MethodMatch; kwargs...)
return specialize_method(match.method, match.spec_types, match.sparams; kwargs...)
end

"""
is_aggressive_constprop(method::Union{Method,CodeInfo}) -> Bool
Check if `method` is declared as `Base.@constprop :aggressive`.
"""
is_aggressive_constprop(method::Union{Method,CodeInfo}) = method.constprop == 0x01

"""
is_no_constprop(method::Union{Method,CodeInfo}) -> Bool
Check if `method` is declared as `Base.@constprop :none`.
"""
is_no_constprop(method::Union{Method,CodeInfo}) = method.constprop == 0x02

#########
# types #
#########
Expand Down
15 changes: 15 additions & 0 deletions src/ast.scm
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,21 @@
(and (if one (length= e 3) (length> e 2))
(eq? (car e) 'meta) (memq (cadr e) '(nospecialize specialize))))

(define (meta? e)
(and (length> e 1) (eq? (car e) 'meta)))

(define (method-meta-sym? x)
(memq x '(inline noinline aggressive_constprop no_constprop propagate_inbounds)))

(define (propagate-method-meta e)
`(meta ,@(filter (lambda (x)
(or (method-meta-sym? x)
(and (pair? x) (eq? (car x) 'purity))))
(cdr e))))

(define (argwide-nospecialize-meta? e)
(and (length= e 2) (eq? (car e) 'meta) (memq (cadr e) '(nospecialize specialize))))

(define (if-generated? e)
(and (length= e 4) (eq? (car e) 'if) (equal? (cadr e) '(generated))))

Expand Down
3 changes: 3 additions & 0 deletions src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,9 @@
,(if (any kwarg? pargl) (gensy) UNUSED)
(call (core kwftype) ,ftype)) ,kw ,@pargl ,@vararg)
`(block
;; propagate method metadata to keyword sorter
,@(map propagate-method-meta (filter meta? prologue))
,@(filter argwide-nospecialize-meta? prologue)
,@(let ((lnns (filter linenum? prologue)))
(if (pair? lnns)
(list (car lnns))
Expand Down
46 changes: 46 additions & 0 deletions test/compiler/inline.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1402,3 +1402,49 @@ end
end
end
end

# https://github.com/JuliaLang/julia/issues/45050
@testset "propagate :meta annotations to keyword sorter methods" begin
# @inline, @noinline, @constprop
let @inline f(::Any; x::Int=1) = 2x
@test ccall(:jl_ir_flag_inlineable, Bool, (Any,), only(methods(f)).source)
@test ccall(:jl_ir_flag_inlineable, Bool, (Any,), only(methods(Core.kwfunc(f))).source)
end
let @noinline f(::Any; x::Int=1) = 2x
@test !ccall(:jl_ir_flag_inlineable, Bool, (Any,), only(methods(f)).source)
@test !ccall(:jl_ir_flag_inlineable, Bool, (Any,), only(methods(Core.kwfunc(f))).source)
end
let Base.@constprop :aggressive f(::Any; x::Int=1) = 2x
@test Core.Compiler.is_aggressive_constprop(only(methods(f)))
@test Core.Compiler.is_aggressive_constprop(only(methods(Core.kwfunc(f))))
end
let Base.@constprop :none f(::Any; x::Int=1) = 2x
@test Core.Compiler.is_no_constprop(only(methods(f)))
@test Core.Compiler.is_no_constprop(only(methods(Core.kwfunc(f))))
end
# @nospecialize
let f(@nospecialize(A::Any); x::Int=1) = 2x
@test only(methods(f)).nospecialize == 1
@test only(methods(Core.kwfunc(f))).nospecialize == 4
end
let f(::Any; x::Int=1) = (@nospecialize; 2x)
@test only(methods(f)).nospecialize == -1
@test only(methods(Core.kwfunc(f))).nospecialize == -1
end
# Base.@assume_effects
let Base.@assume_effects :notaskstate f(::Any; x::Int=1) = 2x
@test Core.Compiler.decode_effects_override(only(methods(f)).purity).notaskstate
@test Core.Compiler.decode_effects_override(only(methods(Core.kwfunc(f))).purity).notaskstate
end
# propagate multiple metadata also
let @inline Base.@assume_effects :notaskstate Base.@constprop :aggressive f(::Any; x::Int=1) = (@nospecialize; 2x)
@test ccall(:jl_ir_flag_inlineable, Bool, (Any,), only(methods(f)).source)
@test Core.Compiler.is_aggressive_constprop(only(methods(f)))
@test ccall(:jl_ir_flag_inlineable, Bool, (Any,), only(methods(Core.kwfunc(f))).source)
@test Core.Compiler.is_aggressive_constprop(only(methods(Core.kwfunc(f))))
@test only(methods(f)).nospecialize == -1
@test only(methods(Core.kwfunc(f))).nospecialize == -1
@test Core.Compiler.decode_effects_override(only(methods(f)).purity).notaskstate
@test Core.Compiler.decode_effects_override(only(methods(Core.kwfunc(f))).purity).notaskstate
end
end

2 comments on commit 9edac22

@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(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 package evaluation job has completed - possible new issues were detected. A full report can be found here.

Please sign in to comment.