Skip to content

Commit d844050

Browse files
committed
Revert "Take purity modeling seriously (#43852)"
This reverts commit f090992.
1 parent 8c26aff commit d844050

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

49 files changed

+186
-1143
lines changed

NEWS.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ Compiler/Runtime improvements
5757
* Abstract callsite can now be inlined or statically resolved as far as the callsite has a single
5858
matching method ([#43113]).
5959
* Builtin function are now a bit more like generic functions, and can be enumerated with `methods` ([#43865]).
60-
* Inference now tracks various effects such as sideeffectful-ness and nothrow-ness on a per-specialization basis. Code heavily dependent on constant propagation should see significant compile-time performance improvements and certain cases (e.g. calls to uninlinable functions that are nevertheless effect free) should see runtime performance improvements. Effects may be overwritten manually with the `@Base.assume_effects` macro. (#43852).
6160

6261
Command-line option changes
6362
---------------------------

base/abstractarray.jl

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1238,16 +1238,10 @@ function unsafe_getindex(A::AbstractArray, I...)
12381238
r
12391239
end
12401240

1241-
struct CanonicalIndexError
1242-
func::String
1243-
type::Any
1244-
CanonicalIndexError(func::String, @nospecialize(type)) = new(func, type)
1245-
end
1246-
12471241
error_if_canonical_getindex(::IndexLinear, A::AbstractArray, ::Int) =
1248-
throw(CanonicalIndexError("getindex", typeof(A)))
1242+
error("getindex not defined for ", typeof(A))
12491243
error_if_canonical_getindex(::IndexCartesian, A::AbstractArray{T,N}, ::Vararg{Int,N}) where {T,N} =
1250-
throw(CanonicalIndexError("getindex", typeof(A)))
1244+
error("getindex not defined for ", typeof(A))
12511245
error_if_canonical_getindex(::IndexStyle, ::AbstractArray, ::Any...) = nothing
12521246

12531247
## Internal definitions
@@ -1339,9 +1333,9 @@ function unsafe_setindex!(A::AbstractArray, v, I...)
13391333
end
13401334

13411335
error_if_canonical_setindex(::IndexLinear, A::AbstractArray, ::Int) =
1342-
throw(CanonicalIndexError("setindex!", typeof(A)))
1336+
error("setindex! not defined for ", typeof(A))
13431337
error_if_canonical_setindex(::IndexCartesian, A::AbstractArray{T,N}, ::Vararg{Int,N}) where {T,N} =
1344-
throw(CanonicalIndexError("setindex!", typeof(A)))
1338+
error("setindex! not defined for ", typeof(A))
13451339
error_if_canonical_setindex(::IndexStyle, ::AbstractArray, ::Any...) = nothing
13461340

13471341
## Internal definitions

base/array.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ function bitsunionsize(u::Union)
213213
end
214214

215215
length(a::Array) = arraylen(a)
216-
elsize(@nospecialize _::Type{A}) where {T,A<:Array{T}} = aligned_sizeof(T)
216+
elsize(::Type{<:Array{T}}) where {T} = aligned_sizeof(T)
217217
sizeof(a::Array) = Core.sizeof(a)
218218

219219
function isassigned(a::Array, i::Int...)

base/boot.jl

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -418,10 +418,9 @@ eval(Core, :(LineInfoNode(mod::Module, @nospecialize(method), file::Symbol, line
418418
$(Expr(:new, :LineInfoNode, :mod, :method, :file, :line, :inlined_at))))
419419
eval(Core, :(CodeInstance(mi::MethodInstance, @nospecialize(rettype), @nospecialize(inferred_const),
420420
@nospecialize(inferred), const_flags::Int32,
421-
min_world::UInt, max_world::UInt, ipo_effects::UInt8, effects::UInt8,
422-
relocatability::UInt8) =
423-
ccall(:jl_new_codeinst, Ref{CodeInstance}, (Any, Any, Any, Any, Int32, UInt, UInt, UInt8, UInt8, UInt8),
424-
mi, rettype, inferred_const, inferred, const_flags, min_world, max_world, ipo_effects, effects, relocatability)))
421+
min_world::UInt, max_world::UInt, relocatability::UInt8) =
422+
ccall(:jl_new_codeinst, Ref{CodeInstance}, (Any, Any, Any, Any, Int32, UInt, UInt, UInt8),
423+
mi, rettype, inferred_const, inferred, const_flags, min_world, max_world, relocatability)))
425424
eval(Core, :(Const(@nospecialize(v)) = $(Expr(:new, :Const, :v))))
426425
eval(Core, :(PartialStruct(@nospecialize(typ), fields::Array{Any, 1}) = $(Expr(:new, :PartialStruct, :typ, :fields))))
427426
eval(Core, :(PartialOpaque(@nospecialize(typ), @nospecialize(env), isva::Bool, parent::MethodInstance, source::Method) = $(Expr(:new, :PartialOpaque, :typ, :env, :isva, :parent, :source))))

base/c.jl

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,3 @@ name, if desired `"libglib-2.0".g_uri_escape_string(...`
733733
macro ccall(expr)
734734
return ccall_macro_lower(:ccall, ccall_macro_parse(expr)...)
735735
end
736-
737-
macro ccall_effects(effects, expr)
738-
return ccall_macro_lower((:ccall, effects), ccall_macro_parse(expr)...)
739-
end

base/compiler/abstractinterpretation.jl

Lines changed: 38 additions & 220 deletions
Large diffs are not rendered by default.

base/compiler/inferencestate.jl

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,6 @@ mutable struct InferenceState
5959
inferred::Bool
6060
dont_work_on_me::Bool
6161

62-
# Inferred purity flags
63-
ipo_effects::Effects
64-
6562
# The place to look up methods while working on this function.
6663
# In particular, we cache method lookup results for the same function to
6764
# fast path repeated queries.
@@ -116,16 +113,6 @@ mutable struct InferenceState
116113
valid_worlds = WorldRange(src.min_world,
117114
src.max_world == typemax(UInt) ? get_world_counter() : src.max_world)
118115

119-
# TODO: Currently, any :inbounds declaration taints consistency,
120-
# because we cannot be guaranteed whether or not boundschecks
121-
# will be eliminated and if they are, we cannot be guaranteed
122-
# that no undefined behavior will occur (the effects assumptions
123-
# are stronger than the inbounds assumptions, since the latter
124-
# requires dynamic reachability, while the former is global).
125-
inbounds = inbounds_option()
126-
inbounds_taints_consistency = !(inbounds === :on || (inbounds === :default && !any_inbounds(code)))
127-
consistent = inbounds_taints_consistency ? TRISTATE_UNKNOWN : ALWAYS_TRUE
128-
129116
@assert cache === :no || cache === :local || cache === :global
130117
frame = new(
131118
params, result, linfo,
@@ -139,26 +126,13 @@ mutable struct InferenceState
139126
Vector{InferenceState}(), # callers_in_cycle
140127
#=parent=#nothing,
141128
cache === :global, false, false,
142-
Effects(consistent, ALWAYS_TRUE, ALWAYS_TRUE, ALWAYS_TRUE,
143-
inbounds_taints_consistency),
144129
CachedMethodTable(method_table(interp)),
145130
interp)
146131
result.result = frame
147132
cache !== :no && push!(get_inference_cache(interp), result)
148133
return frame
149134
end
150135
end
151-
Effects(state::InferenceState) = state.ipo_effects
152-
153-
function any_inbounds(code::Vector{Any})
154-
for i=1:length(code)
155-
stmt = code[i]
156-
if isa(stmt, Expr) && stmt.head === :inbounds
157-
return true
158-
end
159-
end
160-
return false
161-
end
162136

163137
function compute_trycatch(code::Vector{Any}, ip::BitSet)
164138
# The goal initially is to record the frame like this for the state at exit:

base/compiler/optimize.jl

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,16 @@ const IR_FLAG_THROW_BLOCK = 0x01 << 3
149149
# thus be both pure and effect free.
150150
const IR_FLAG_EFFECT_FREE = 0x01 << 4
151151

152+
# known to be always effect-free (in particular nothrow)
153+
const _PURE_BUILTINS = Any[tuple, svec, ===, typeof, nfields]
154+
155+
# known to be effect-free if the are nothrow
156+
const _PURE_OR_ERROR_BUILTINS = [
157+
fieldtype, apply_type, isa, UnionAll,
158+
getfield, arrayref, const_arrayref, arraysize, isdefined, Core.sizeof,
159+
Core.kwfunc, Core.ifelse, Core._typevar, (<:),
160+
]
161+
152162
const TOP_TUPLE = GlobalRef(Core, :tuple)
153163

154164
#########
@@ -215,7 +225,7 @@ function stmt_effect_free(@nospecialize(stmt), @nospecialize(rt), src::Union{IRC
215225
M, s = argextype(args[2], src), argextype(args[3], src)
216226
return get_binding_type_effect_free(M, s)
217227
end
218-
contains_is(_EFFECT_FREE_BUILTINS, f) || return false
228+
contains_is(_PURE_OR_ERROR_BUILTINS, f) || return false
219229
rt === Bottom && return false
220230
return _builtin_nothrow(f, Any[argextype(args[i], src) for i = 2:length(args)], rt)
221231
elseif head === :new
@@ -287,14 +297,12 @@ function alloc_array_ndims(name::Symbol)
287297
return nothing
288298
end
289299

290-
const FOREIGNCALL_ARG_START = 6
291-
292300
function alloc_array_no_throw(args::Vector{Any}, ndims::Int, src::Union{IRCode,IncrementalCompact})
293-
length(args) ndims+FOREIGNCALL_ARG_START || return false
294-
atype = instanceof_tfunc(argextype(args[FOREIGNCALL_ARG_START], src))[1]
301+
length(args) ndims+6 || return false
302+
atype = instanceof_tfunc(argextype(args[6], src))[1]
295303
dims = Csize_t[]
296304
for i in 1:ndims
297-
dim = argextype(args[i+FOREIGNCALL_ARG_START], src)
305+
dim = argextype(args[i+6], src)
298306
isa(dim, Const) || return false
299307
dimval = dim.val
300308
isa(dimval, Int) || return false
@@ -304,9 +312,9 @@ function alloc_array_no_throw(args::Vector{Any}, ndims::Int, src::Union{IRCode,I
304312
end
305313

306314
function new_array_no_throw(args::Vector{Any}, src::Union{IRCode,IncrementalCompact})
307-
length(args) FOREIGNCALL_ARG_START+1 || return false
308-
atype = instanceof_tfunc(argextype(args[FOREIGNCALL_ARG_START], src))[1]
309-
dims = argextype(args[FOREIGNCALL_ARG_START+1], src)
315+
length(args) 7 || return false
316+
atype = instanceof_tfunc(argextype(args[6], src))[1]
317+
dims = argextype(args[7], src)
310318
isa(dims, Const) || return dims === Tuple{}
311319
dimsval = dims.val
312320
isa(dimsval, Tuple{Vararg{Int}}) || return false
@@ -613,6 +621,21 @@ function slot2reg(ir::IRCode, ci::CodeInfo, sv::OptimizationState)
613621
return ir
614622
end
615623

624+
# whether `f` is pure for inference
625+
function is_pure_intrinsic_infer(f::IntrinsicFunction)
626+
return !(f === Intrinsics.pointerref || # this one is volatile
627+
f === Intrinsics.pointerset || # this one is never effect-free
628+
f === Intrinsics.llvmcall || # this one is never effect-free
629+
f === Intrinsics.arraylen || # this one is volatile
630+
f === Intrinsics.sqrt_llvm_fast || # this one may differ at runtime (by a few ulps)
631+
f === Intrinsics.have_fma || # this one depends on the runtime environment
632+
f === Intrinsics.cglobal) # cglobal lookup answer changes at runtime
633+
end
634+
635+
# whether `f` is effect free if nothrow
636+
intrinsic_effect_free_if_nothrow(f) = f === Intrinsics.pointerref ||
637+
f === Intrinsics.have_fma || is_pure_intrinsic_infer(f)
638+
616639
## Computing the cost of a function body
617640

618641
# saturating sum (inputs are nonnegative), prevents overflow with typemax(Int) below

0 commit comments

Comments
 (0)