From b77a6e22504f74a87265447a1f7289ef023d3e5b Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Sun, 5 Sep 2021 05:51:45 -0500 Subject: [PATCH] Allow constant-propagation to be disabled Our heuristics for constant propagation are imperfect (and probably never will be perfect), and I've now seen many examples of methods that no developer would ask to have const-propped get that treatment. In some cases the cost for latency/precompilation is very large. This renames `@aggressive_constprop` to `@constprop` and allows two settings, `:aggressive` and `:none`. Closes #38983 (presuming this will propagate to kw body methods) --- base/char.jl | 24 +++++++++--------- base/compiler/abstractinterpretation.jl | 3 ++- base/expr.jl | 30 +++++++++++++++-------- src/ast.c | 3 ++- src/dump.c | 4 +-- src/ircode.c | 4 +-- src/jltypes.c | 8 +++--- src/julia.h | 6 +++-- src/julia_internal.h | 2 +- src/method.c | 10 +++++--- stdlib/Serialization/src/Serialization.jl | 10 ++++---- test/compiler/inference.jl | 10 ++++++-- test/compiler/inline.jl | 12 ++++----- 13 files changed, 74 insertions(+), 52 deletions(-) diff --git a/base/char.jl b/base/char.jl index 0584471cb6a33..c8b1c28166bbf 100644 --- a/base/char.jl +++ b/base/char.jl @@ -45,10 +45,10 @@ represents a valid Unicode character. """ Char -@aggressive_constprop (::Type{T})(x::Number) where {T<:AbstractChar} = T(UInt32(x)) -@aggressive_constprop AbstractChar(x::Number) = Char(x) -@aggressive_constprop (::Type{T})(x::AbstractChar) where {T<:Union{Number,AbstractChar}} = T(codepoint(x)) -@aggressive_constprop (::Type{T})(x::AbstractChar) where {T<:Union{Int32,Int64}} = codepoint(x) % T +@constprop :aggressive (::Type{T})(x::Number) where {T<:AbstractChar} = T(UInt32(x)) +@constprop :aggressive AbstractChar(x::Number) = Char(x) +@constprop :aggressive (::Type{T})(x::AbstractChar) where {T<:Union{Number,AbstractChar}} = T(codepoint(x)) +@constprop :aggressive (::Type{T})(x::AbstractChar) where {T<:Union{Int32,Int64}} = codepoint(x) % T (::Type{T})(x::T) where {T<:AbstractChar} = x """ @@ -75,7 +75,7 @@ return a different-sized integer (e.g. `UInt8`). """ function codepoint end -@aggressive_constprop codepoint(c::Char) = UInt32(c) +@constprop :aggressive codepoint(c::Char) = UInt32(c) struct InvalidCharError{T<:AbstractChar} <: Exception char::T @@ -124,7 +124,7 @@ See also [`decode_overlong`](@ref) and [`show_invalid`](@ref). """ isoverlong(c::AbstractChar) = false -@aggressive_constprop function UInt32(c::Char) +@constprop :aggressive function UInt32(c::Char) # TODO: use optimized inline LLVM u = bitcast(UInt32, c) u < 0x80000000 && return u >> 24 @@ -148,7 +148,7 @@ that support overlong encodings should implement `Base.decode_overlong`. """ function decode_overlong end -@aggressive_constprop function decode_overlong(c::Char) +@constprop :aggressive function decode_overlong(c::Char) u = bitcast(UInt32, c) l1 = leading_ones(u) t0 = trailing_zeros(u) & 56 @@ -158,7 +158,7 @@ function decode_overlong end ((u & 0x007f0000) >> 4) | ((u & 0x7f000000) >> 6) end -@aggressive_constprop function Char(u::UInt32) +@constprop :aggressive function Char(u::UInt32) u < 0x80 && return bitcast(Char, u << 24) u < 0x00200000 || throw_code_point_err(u) c = ((u << 0) & 0x0000003f) | ((u << 2) & 0x00003f00) | @@ -169,14 +169,14 @@ end bitcast(Char, c) end -@aggressive_constprop @noinline UInt32_cold(c::Char) = UInt32(c) -@aggressive_constprop function (T::Union{Type{Int8},Type{UInt8}})(c::Char) +@constprop :aggressive @noinline UInt32_cold(c::Char) = UInt32(c) +@constprop :aggressive function (T::Union{Type{Int8},Type{UInt8}})(c::Char) i = bitcast(Int32, c) i ≥ 0 ? ((i >>> 24) % T) : T(UInt32_cold(c)) end -@aggressive_constprop @noinline Char_cold(b::UInt32) = Char(b) -@aggressive_constprop function Char(b::Union{Int8,UInt8}) +@constprop :aggressive @noinline Char_cold(b::UInt32) = Char(b) +@constprop :aggressive function Char(b::Union{Int8,UInt8}) 0 ≤ b ≤ 0x7f ? bitcast(Char, (b % UInt32) << 24) : Char_cold(UInt32(b)) end diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index 2bb3943a3f83f..dfac52f5767a1 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -568,6 +568,7 @@ function maybe_get_const_prop_profitable(interp::AbstractInterpreter, result::Me return nothing end method = match.method + method.constprop == 0x02 && return nothing force = force_const_prop(interp, f, method) force || const_prop_entry_heuristic(interp, result, sv) || return nothing nargs::Int = method.nargs @@ -649,7 +650,7 @@ function is_allconst(argtypes::Vector{Any}) end function force_const_prop(interp::AbstractInterpreter, @nospecialize(f), method::Method) - return method.aggressive_constprop || + return method.constprop == 0x01 || InferenceParams(interp).aggressive_constant_propagation || istopfunction(f, :getproperty) || istopfunction(f, :setproperty!) diff --git a/base/expr.jl b/base/expr.jl index 1af1e9486068e..1d95ae73c9cb3 100644 --- a/base/expr.jl +++ b/base/expr.jl @@ -349,16 +349,26 @@ macro pure(ex) end """ - @aggressive_constprop ex - @aggressive_constprop(ex) - -`@aggressive_constprop` requests more aggressive interprocedural constant -propagation for the annotated function. For a method where the return type -depends on the value of the arguments, this can yield improved inference results -at the cost of additional compile time. -""" -macro aggressive_constprop(ex) - esc(isa(ex, Expr) ? pushmeta!(ex, :aggressive_constprop) : ex) + @constprop setting ex + @constprop(setting, ex) + +`@constprop` controls the mode of interprocedural constant propagation for the +annotated function. Two `setting`s are supported: + +- `@constprop :aggressive ex`: apply constant propagation aggressively. + For a method where the return type depends on the value of the arguments, + this can yield improved inference results at the cost of additional compile time. +- `@constprop :none ex`: disable constant propagation. This can reduce compile + times for functions that Julia might otherwise deem worthy of constant-propagation. + Common cases are for functions with `Bool`- or `Symbol`-valued arguments or keyword arguments. +""" +macro constprop(setting, ex) + if isa(setting, QuoteNode) + setting = setting.value + end + setting === :aggressive && return esc(isa(ex, Expr) ? pushmeta!(ex, :aggressive_constprop) : ex) + setting === :none && return esc(isa(ex, Expr) ? pushmeta!(ex, :no_constprop) : ex) + throw(ArgumentError("@constprop $setting not supported")) end """ diff --git a/src/ast.c b/src/ast.c index c33bf56d91379..05a0a403ccc58 100644 --- a/src/ast.c +++ b/src/ast.c @@ -59,7 +59,7 @@ jl_sym_t *static_parameter_sym; jl_sym_t *inline_sym; jl_sym_t *noinline_sym; jl_sym_t *generated_sym; jl_sym_t *generated_only_sym; jl_sym_t *isdefined_sym; jl_sym_t *propagate_inbounds_sym; jl_sym_t *specialize_sym; -jl_sym_t *aggressive_constprop_sym; +jl_sym_t *aggressive_constprop_sym; jl_sym_t *no_constprop_sym; jl_sym_t *nospecialize_sym; jl_sym_t *macrocall_sym; jl_sym_t *colon_sym; jl_sym_t *hygienicscope_sym; jl_sym_t *throw_undef_if_not_sym; jl_sym_t *getfield_undefref_sym; @@ -399,6 +399,7 @@ void jl_init_common_symbols(void) polly_sym = jl_symbol("polly"); propagate_inbounds_sym = jl_symbol("propagate_inbounds"); aggressive_constprop_sym = jl_symbol("aggressive_constprop"); + no_constprop_sym = jl_symbol("no_constprop"); isdefined_sym = jl_symbol("isdefined"); nospecialize_sym = jl_symbol("nospecialize"); specialize_sym = jl_symbol("specialize"); diff --git a/src/dump.c b/src/dump.c index afadca3edad2a..20d3f5689d353 100644 --- a/src/dump.c +++ b/src/dump.c @@ -671,7 +671,7 @@ static void jl_serialize_value_(jl_serializer_state *s, jl_value_t *v, int as_li write_int8(s->s, m->isva); write_int8(s->s, m->pure); write_int8(s->s, m->is_for_opaque_closure); - write_int8(s->s, m->aggressive_constprop); + write_int8(s->s, m->constprop); jl_serialize_value(s, (jl_value_t*)m->slot_syms); jl_serialize_value(s, (jl_value_t*)m->roots); jl_serialize_value(s, (jl_value_t*)m->ccallable); @@ -1525,7 +1525,7 @@ static jl_value_t *jl_deserialize_value_method(jl_serializer_state *s, jl_value_ m->isva = read_int8(s->s); m->pure = read_int8(s->s); m->is_for_opaque_closure = read_int8(s->s); - m->aggressive_constprop = read_int8(s->s); + m->constprop = read_int8(s->s); m->slot_syms = jl_deserialize_value(s, (jl_value_t**)&m->slot_syms); jl_gc_wb(m, m->slot_syms); m->roots = (jl_array_t*)jl_deserialize_value(s, (jl_value_t**)&m->roots); diff --git a/src/ircode.c b/src/ircode.c index 212febe121a75..62c814b769dd1 100644 --- a/src/ircode.c +++ b/src/ircode.c @@ -702,7 +702,7 @@ JL_DLLEXPORT jl_array_t *jl_compress_ir(jl_method_t *m, jl_code_info_t *code) jl_current_task->ptls }; - uint8_t flags = (code->aggressive_constprop << 4) + uint8_t flags = (code->constprop << 4) // this consumes two bits: 4 = aggressive, 5 = none | (code->inferred << 3) | (code->inlineable << 2) | (code->propagate_inbounds << 1) @@ -788,7 +788,7 @@ JL_DLLEXPORT jl_code_info_t *jl_uncompress_ir(jl_method_t *m, jl_code_instance_t jl_code_info_t *code = jl_new_code_info_uninit(); uint8_t flags = read_uint8(s.s); - code->aggressive_constprop = !!(flags & (1 << 4)); + code->constprop = !!(flags & (3 << 4)); code->inferred = !!(flags & (1 << 3)); code->inlineable = !!(flags & (1 << 2)); code->propagate_inbounds = !!(flags & (1 << 1)); diff --git a/src/jltypes.c b/src/jltypes.c index 43171ee332e87..1330650ee8892 100644 --- a/src/jltypes.c +++ b/src/jltypes.c @@ -2348,7 +2348,7 @@ void jl_init_types(void) JL_GC_DISABLED "inlineable", "propagate_inbounds", "pure", - "aggressive_constprop"), + "constprop"), jl_svec(19, jl_array_any_type, jl_array_int32_type, @@ -2368,7 +2368,7 @@ void jl_init_types(void) JL_GC_DISABLED jl_bool_type, jl_bool_type, jl_bool_type, - jl_bool_type), + jl_uint8_type), jl_emptysvec, 0, 1, 19); @@ -2401,7 +2401,7 @@ void jl_init_types(void) JL_GC_DISABLED "isva", "pure", "is_for_opaque_closure", - "aggressive_constprop"), + "constprop"), jl_svec(26, jl_symbol_type, jl_module_type, @@ -2428,7 +2428,7 @@ void jl_init_types(void) JL_GC_DISABLED jl_bool_type, jl_bool_type, jl_bool_type, - jl_bool_type), + jl_uint8_type), jl_emptysvec, 0, 1, 10); diff --git a/src/julia.h b/src/julia.h index e53b33bef674d..097e247861ad3 100644 --- a/src/julia.h +++ b/src/julia.h @@ -278,7 +278,8 @@ typedef struct _jl_code_info_t { uint8_t inlineable; uint8_t propagate_inbounds; uint8_t pure; - uint8_t aggressive_constprop; + // uint8 settings + uint8_t constprop; // 0x00 = use heuristic; 0x01 = aggressive; 0x02 = none } jl_code_info_t; // This type describes a single method definition, and stores data @@ -326,7 +327,8 @@ typedef struct _jl_method_t { uint8_t isva; uint8_t pure; uint8_t is_for_opaque_closure; - uint8_t aggressive_constprop; + // uint8 settings + uint8_t constprop; // 0x00 = use heuristic; 0x01 = aggressive; 0x02 = none // hidden fields: // lock for modifications to the method diff --git a/src/julia_internal.h b/src/julia_internal.h index c0d492241cf0d..5e9b03d4aa7d4 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -1376,7 +1376,7 @@ extern jl_sym_t *static_parameter_sym; extern jl_sym_t *inline_sym; extern jl_sym_t *noinline_sym; extern jl_sym_t *generated_sym; extern jl_sym_t *generated_only_sym; extern jl_sym_t *isdefined_sym; extern jl_sym_t *propagate_inbounds_sym; extern jl_sym_t *specialize_sym; -extern jl_sym_t *aggressive_constprop_sym; +extern jl_sym_t *aggressive_constprop_sym; extern jl_sym_t *no_constprop_sym; extern jl_sym_t *nospecialize_sym; extern jl_sym_t *macrocall_sym; extern jl_sym_t *colon_sym; extern jl_sym_t *hygienicscope_sym; extern jl_sym_t *throw_undef_if_not_sym; extern jl_sym_t *getfield_undefref_sym; diff --git a/src/method.c b/src/method.c index c17566ac6f936..32011cab1fd82 100644 --- a/src/method.c +++ b/src/method.c @@ -303,7 +303,9 @@ static void jl_code_info_set_ir(jl_code_info_t *li, jl_expr_t *ir) else if (ma == (jl_value_t*)propagate_inbounds_sym) li->propagate_inbounds = 1; else if (ma == (jl_value_t*)aggressive_constprop_sym) - li->aggressive_constprop = 1; + li->constprop = 1; + else if (ma == (jl_value_t*)no_constprop_sym) + li->constprop = 2; else jl_array_ptr_set(meta, ins++, ma); } @@ -443,7 +445,7 @@ JL_DLLEXPORT jl_code_info_t *jl_new_code_info_uninit(void) src->propagate_inbounds = 0; src->pure = 0; src->edges = jl_nothing; - src->aggressive_constprop = 0; + src->constprop = 0; return src; } @@ -630,7 +632,7 @@ static void jl_method_set_source(jl_method_t *m, jl_code_info_t *src) } m->called = called; m->pure = src->pure; - m->aggressive_constprop = src->aggressive_constprop; + m->constprop = src->constprop; jl_add_function_name_to_lineinfo(src, (jl_value_t*)m->name); jl_array_t *copy = NULL; @@ -746,7 +748,7 @@ JL_DLLEXPORT jl_method_t *jl_new_method_uninit(jl_module_t *module) m->primary_world = 1; m->deleted_world = ~(size_t)0; m->is_for_opaque_closure = 0; - m->aggressive_constprop = 0; + m->constprop = 0; JL_MUTEX_INIT(&m->writelock); return m; } diff --git a/stdlib/Serialization/src/Serialization.jl b/stdlib/Serialization/src/Serialization.jl index 592db96565c7a..20eb631323c1e 100644 --- a/stdlib/Serialization/src/Serialization.jl +++ b/stdlib/Serialization/src/Serialization.jl @@ -418,7 +418,7 @@ function serialize(s::AbstractSerializer, meth::Method) serialize(s, meth.nargs) serialize(s, meth.isva) serialize(s, meth.is_for_opaque_closure) - serialize(s, meth.aggressive_constprop) + serialize(s, meth.constprop) if isdefined(meth, :source) serialize(s, Base._uncompressed_ast(meth, meth.source)) else @@ -1014,12 +1014,12 @@ function deserialize(s::AbstractSerializer, ::Type{Method}) nargs = deserialize(s)::Int32 isva = deserialize(s)::Bool is_for_opaque_closure = false - aggressive_constprop = false + constprop = 0x00 template_or_is_opaque = deserialize(s) if isa(template_or_is_opaque, Bool) is_for_opaque_closure = template_or_is_opaque if format_version(s) >= 14 - aggressive_constprop = deserialize(s)::Bool + constprop = deserialize(s)::UInt8 end template = deserialize(s) else @@ -1039,7 +1039,7 @@ function deserialize(s::AbstractSerializer, ::Type{Method}) meth.nargs = nargs meth.isva = isva meth.is_for_opaque_closure = is_for_opaque_closure - meth.aggressive_constprop = aggressive_constprop + meth.constprop = constprop if template !== nothing # TODO: compress template meth.source = template::CodeInfo @@ -1163,7 +1163,7 @@ function deserialize(s::AbstractSerializer, ::Type{CodeInfo}) ci.propagate_inbounds = deserialize(s) ci.pure = deserialize(s) if format_version(s) >= 14 - ci.aggressive_constprop = deserialize(s)::Bool + ci.constprop = deserialize(s)::UInt8 end return ci end diff --git a/test/compiler/inference.jl b/test/compiler/inference.jl index 2b85fcbfc943e..e5e83ac612fe7 100644 --- a/test/compiler/inference.jl +++ b/test/compiler/inference.jl @@ -3168,9 +3168,9 @@ g38888() = S38888(Base.inferencebarrier(3), nothing) f_inf_error_bottom(x::Vector) = isempty(x) ? error(x[1]) : x @test Core.Compiler.return_type(f_inf_error_bottom, Tuple{Vector{Any}}) == Vector{Any} -# @aggressive_constprop +# @constprop :aggressive @noinline g_nonaggressive(y, x) = Val{x}() -@noinline @Base.aggressive_constprop g_aggressive(y, x) = Val{x}() +@noinline Base.@constprop :aggressive g_aggressive(y, x) = Val{x}() f_nonaggressive(x) = g_nonaggressive(x, 1) f_aggressive(x) = g_aggressive(x, 1) @@ -3180,6 +3180,12 @@ f_aggressive(x) = g_aggressive(x, 1) @test Base.return_types(f_nonaggressive, Tuple{Int})[1] == Val @test Base.return_types(f_aggressive, Tuple{Int})[1] == Val{1} +# @constprop :none +@noinline Base.@constprop :none g_noaggressive(flag::Bool) = flag ? 1 : 1.0 +ftrue_noaggressive() = g_noaggressive(true) +@test only(Base.return_types(ftrue_noaggressive, Tuple{})) == Union{Int,Float64} + + function splat_lotta_unions() a = Union{Tuple{Int},Tuple{String,Vararg{Int}},Tuple{Int,Vararg{Int}}}[(2,)][1] b = Union{Int8,Int16,Int32,Int64,Int128}[1][1] diff --git a/test/compiler/inline.jl b/test/compiler/inline.jl index efe34eb5b35d9..a276e6ef65c77 100644 --- a/test/compiler/inline.jl +++ b/test/compiler/inline.jl @@ -403,13 +403,13 @@ code_typed1(args...; kwargs...) = (first(only(code_typed(args...; kwargs...))):: def_noinline(x) = _def_noinline(x) # test that they don't conflict with other "before-definition" macros - @inline Base.@aggressive_constprop function _def_inline_noconflict(x) + @inline Base.@constprop :aggressive function _def_inline_noconflict(x) # this call won't be resolved and thus will prevent inlining to happen if we don't # annotate `@inline` at the top of this function body return unresolved_call(x) end def_inline_noconflict(x) = _def_inline_noconflict(x) - @noinline Base.@aggressive_constprop _def_noinline_noconflict(x) = x # obviously will be inlined otherwise + @noinline Base.@constprop :aggressive _def_noinline_noconflict(x) = x # obviously will be inlined otherwise def_noinline_noconflict(x) = _def_noinline_noconflict(x) end @@ -519,14 +519,14 @@ end # test callsite annotations for constant-prop'ed calls - @noinline Base.@aggressive_constprop noinlined_constprop_explicit(a) = a+g + @noinline Base.@constprop :aggressive noinlined_constprop_explicit(a) = a+g force_inline_constprop_explicit() = @inline noinlined_constprop_explicit(0) - Base.@aggressive_constprop noinlined_constprop_implicit(a) = a+g + Base.@constprop :aggressive noinlined_constprop_implicit(a) = a+g force_inline_constprop_implicit() = @inline noinlined_constprop_implicit(0) - @inline Base.@aggressive_constprop inlined_constprop_explicit(a) = a+g + @inline Base.@constprop :aggressive inlined_constprop_explicit(a) = a+g force_noinline_constprop_explicit() = @noinline inlined_constprop_explicit(0) - @inline Base.@aggressive_constprop inlined_constprop_implicit(a) = a+g + @inline Base.@constprop :aggressive inlined_constprop_implicit(a) = a+g force_noinline_constprop_implicit() = @noinline inlined_constprop_implicit(0) @noinline notinlined(a) = a