Skip to content

Commit f0c7c24

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 896d0b1 commit f0c7c24

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
# Types compatible with fpext/fptrunc
24332429
const CORE_FLOAT_TYPES = Union{Core.BFloat16, Float16, Float32, Float64}
24342430

@@ -2498,6 +2494,71 @@ function getfield_effects(𝕃::AbstractLattice, argtypes::Vector{Any}, @nospeci
24982494
return Effects(EFFECTS_TOTAL; consistent, nothrow, inaccessiblememonly, noub)
24992495
end
25002496

2497+
# add a new builtin function to this list only after making sure that
2498+
# `builtin_effects` is properly implemented for it
2499+
const _EFFECTS_KNOWN_BUILTINS = Any[
2500+
<:,
2501+
===,
2502+
# Core._abstracttype,
2503+
# _apply_iterate,
2504+
# Core._call_in_world_total,
2505+
# Core._compute_sparams,
2506+
# Core._defaultctors,
2507+
# Core._equiv_typedef,
2508+
Core._expr,
2509+
# Core._primitivetype,
2510+
# Core._setsuper!,
2511+
# Core._structtype,
2512+
# Core._svec_ref,
2513+
# Core._typebody!,
2514+
Core._typevar,
2515+
apply_type,
2516+
compilerbarrier,
2517+
Core.current_scope,
2518+
donotdelete,
2519+
Core.finalizer,
2520+
Core.get_binding_type,
2521+
Core.ifelse,
2522+
# Core.invoke_in_world,
2523+
# invokelatest,
2524+
Core.memorynew,
2525+
memoryref_isassigned,
2526+
memoryrefget,
2527+
# Core.memoryrefmodify!,
2528+
memoryrefnew,
2529+
memoryrefoffset,
2530+
# Core.memoryrefreplace!,
2531+
memoryrefset!,
2532+
# Core.memoryrefsetonce!,
2533+
# Core.memoryrefswap!,
2534+
Core.sizeof,
2535+
svec,
2536+
Core.throw_methoderror,
2537+
applicable,
2538+
fieldtype,
2539+
getfield,
2540+
getglobal,
2541+
# invoke,
2542+
isa,
2543+
isdefined,
2544+
# isdefinedglobal,
2545+
modifyfield!,
2546+
# modifyglobal!,
2547+
nfields,
2548+
replacefield!,
2549+
# replaceglobal!,
2550+
setfield!,
2551+
# setfieldonce!,
2552+
# setglobal!,
2553+
# setglobalonce!,
2554+
swapfield!,
2555+
# swapglobal!,
2556+
throw,
2557+
tuple,
2558+
typeassert,
2559+
typeof
2560+
]
2561+
25012562
"""
25022563
builtin_effects(𝕃::AbstractLattice, f::Builtin, argtypes::Vector{Any}, rt) -> Effects
25032564
@@ -2508,7 +2569,9 @@ function builtin_effects(𝕃::AbstractLattice, @nospecialize(f::Builtin), argty
25082569
return intrinsic_effects(f, argtypes)
25092570
end
25102571

2511-
@assert !contains_is(_SPECIAL_BUILTINS, f)
2572+
if !(f in _EFFECTS_KNOWN_BUILTINS)
2573+
return Effects()
2574+
end
25122575

25132576
if f === getfield
25142577
return getfield_effects(𝕃, argtypes, rt)

Compiler/test/effects.jl

+6
Original file line numberDiff line numberDiff line change
@@ -1435,3 +1435,9 @@ end
14351435
let effects = Base.infer_effects(Base._unsetindex!, (MemoryRef{String},))
14361436
@test !Compiler.is_effect_free(effects)
14371437
end
1438+
1439+
# builtin functions that can do arbitrary things should have the top effects
1440+
@test Base.infer_effects(Core._call_in_world_total, Tuple{Vararg{Any}}) == Compiler.Effects()
1441+
@test Base.infer_effects(Core.invoke_in_world, Tuple{Vararg{Any}}) == Compiler.Effects()
1442+
@test Base.infer_effects(invokelatest, Tuple{Vararg{Any}}) == Compiler.Effects()
1443+
@test Base.infer_effects(invoke, Tuple{Vararg{Any}}) == Compiler.Effects()

0 commit comments

Comments
 (0)