Skip to content

Commit af154ee

Browse files
committed
effects: give the most conservative effects to unhandled builtins
This commit fixes a bug where `builtin_tfunction` was giving the wrong effects to builtins that weren't actually implemented. xref: #57806 (comment) Now, when we add new builtins, unless we manually update `_EFFECTS_KNOWN_BUILTINS`, they'll automatically get the most conservative `Effects()`. Effects for builtins like `getglobal` are calculated individually inside `abstract_call_builtin`, so there is no handling for them within `builtin_effects` and thus they aren't in `_EFFECTS_KNOWN_BUILTINS`. If we want to give these built-ins a better `stmt_effect_flag`, we need to handle it in `builtin_effects` properly.
1 parent bd67367 commit af154ee

File tree

2 files changed

+74
-5
lines changed

2 files changed

+74
-5
lines changed

Compiler/src/tfuncs.jl

+68-5
Original file line numberDiff line numberDiff line change
@@ -2425,10 +2425,6 @@ const _INCONSISTENT_INTRINSICS = Any[
24252425
# Intrinsics.muladd_float, # this is not interprocedurally consistent
24262426
]
24272427

2428-
const _SPECIAL_BUILTINS = Any[
2429-
Core._apply_iterate,
2430-
]
2431-
24322428
# Intrinsics that require all arguments to be floats
24332429
const _FLOAT_INTRINSICS = Any[
24342430
Intrinsics.neg_float,
@@ -2535,6 +2531,71 @@ function getfield_effects(𝕃::AbstractLattice, argtypes::Vector{Any}, @nospeci
25352531
return Effects(EFFECTS_TOTAL; consistent, nothrow, inaccessiblememonly, noub)
25362532
end
25372533

2534+
# add a new builtin function to this list only after making sure that
2535+
# `builtin_effects` is properly implemented for it
2536+
const _EFFECTS_KNOWN_BUILTINS = Any[
2537+
<:,
2538+
===,
2539+
# Core._abstracttype,
2540+
# _apply_iterate,
2541+
# Core._call_in_world_total,
2542+
# Core._compute_sparams,
2543+
# Core._defaultctors,
2544+
# Core._equiv_typedef,
2545+
Core._expr,
2546+
# Core._primitivetype,
2547+
# Core._setsuper!,
2548+
# Core._structtype,
2549+
# Core._svec_ref,
2550+
# Core._typebody!,
2551+
Core._typevar,
2552+
apply_type,
2553+
compilerbarrier,
2554+
Core.current_scope,
2555+
donotdelete,
2556+
Core.finalizer,
2557+
Core.get_binding_type,
2558+
Core.ifelse,
2559+
# Core.invoke_in_world,
2560+
# invokelatest,
2561+
Core.memorynew,
2562+
memoryref_isassigned,
2563+
memoryrefget,
2564+
# Core.memoryrefmodify!,
2565+
memoryrefnew,
2566+
memoryrefoffset,
2567+
# Core.memoryrefreplace!,
2568+
memoryrefset!,
2569+
# Core.memoryrefsetonce!,
2570+
# Core.memoryrefswap!,
2571+
Core.sizeof,
2572+
svec,
2573+
Core.throw_methoderror,
2574+
applicable,
2575+
fieldtype,
2576+
getfield,
2577+
getglobal,
2578+
# invoke,
2579+
isa,
2580+
isdefined,
2581+
# isdefinedglobal,
2582+
modifyfield!,
2583+
# modifyglobal!,
2584+
nfields,
2585+
replacefield!,
2586+
# replaceglobal!,
2587+
setfield!,
2588+
# setfieldonce!,
2589+
# setglobal!,
2590+
# setglobalonce!,
2591+
swapfield!,
2592+
# swapglobal!,
2593+
throw,
2594+
tuple,
2595+
typeassert,
2596+
typeof
2597+
]
2598+
25382599
"""
25392600
builtin_effects(𝕃::AbstractLattice, f::Builtin, argtypes::Vector{Any}, rt) -> Effects
25402601
@@ -2545,7 +2606,9 @@ function builtin_effects(𝕃::AbstractLattice, @nospecialize(f::Builtin), argty
25452606
return intrinsic_effects(f, argtypes)
25462607
end
25472608

2548-
@assert !contains_is(_SPECIAL_BUILTINS, f)
2609+
if !(f in _EFFECTS_KNOWN_BUILTINS)
2610+
return Effects()
2611+
end
25492612

25502613
if f === getfield
25512614
return getfield_effects(𝕃, argtypes, rt)

Compiler/test/effects.jl

+6
Original file line numberDiff line numberDiff line change
@@ -1461,3 +1461,9 @@ end
14611461
let effects = Base.infer_effects(Base._unsetindex!, (MemoryRef{String},))
14621462
@test !Compiler.is_effect_free(effects)
14631463
end
1464+
1465+
# builtin functions that can do arbitrary things should have the top effects
1466+
@test Base.infer_effects(Core._call_in_world_total, Tuple{Vararg{Any}}) == Compiler.Effects()
1467+
@test Base.infer_effects(Core.invoke_in_world, Tuple{Vararg{Any}}) == Compiler.Effects()
1468+
@test Base.infer_effects(invokelatest, Tuple{Vararg{Any}}) == Compiler.Effects()
1469+
@test Base.infer_effects(invoke, Tuple{Vararg{Any}}) == Compiler.Effects()

0 commit comments

Comments
 (0)