Skip to content

Commit c70094e

Browse files
KenoKristofferC
authored andcommitted
Put back getfield :boundscheck hack (#48677)
We used to have this hack before #48246, but I removed it because I had hoped we don't need. Unfortunately, we weren't able to infer consistency of ``` @inbounds (1,2)[1] ``` With this hack, constprop is able to independently prove inbounded-ness, overriding the usual consistency taintaing that happens for inbounds. (cherry picked from commit 113c2f3)
1 parent 5e39fc6 commit c70094e

File tree

2 files changed

+25
-2
lines changed

2 files changed

+25
-2
lines changed

base/compiler/abstractinterpretation.jl

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1837,6 +1837,14 @@ function abstract_call_known(interp::AbstractInterpreter, @nospecialize(f),
18371837
end
18381838
rt = abstract_call_builtin(interp, f, arginfo, sv, max_methods)
18391839
effects = builtin_effects(typeinf_lattice(interp), f, argtypes[2:end], rt)
1840+
if f === getfield && (fargs !== nothing && isexpr(fargs[end], :boundscheck)) && !is_nothrow(effects) && isa(sv, InferenceState)
1841+
# As a special case, we delayed tainting `noinbounds` for getfield calls in case we can prove
1842+
# in-boundedness indepedently. Here we need to put that back in other cases.
1843+
# N.B.: This isn't about the effects of the call itself, but a delayed contribution of the :boundscheck
1844+
# statement, so we need to merge this directly into sv, rather than modifying thte effects.
1845+
merge_effects!(interp, sv, Effects(EFFECTS_TOTAL; noinbounds=false,
1846+
consistent = (get_curr_ssaflag(sv) & IR_FLAG_INBOUNDS) != 0 ? ALWAYS_FALSE : ALWAYS_TRUE))
1847+
end
18401848
return CallMeta(rt, effects, NoCallInfo())
18411849
elseif isa(f, Core.OpaqueClosure)
18421850
# calling an OpaqueClosure about which we have no information returns no information
@@ -2045,6 +2053,15 @@ function abstract_eval_value_expr(interp::AbstractInterpreter, e::Expr, vtypes::
20452053
end
20462054
elseif head === :boundscheck
20472055
if isa(sv, InferenceState)
2056+
stmt = sv.src.code[sv.currpc]
2057+
if isexpr(stmt, :call)
2058+
f = abstract_eval_value(interp, stmt.args[1], vtypes, sv)
2059+
if f isa Const && f.val === getfield
2060+
# boundscheck of `getfield` call is analyzed by tfunc potentially without
2061+
# tainting :inbounds or :consistent when it's known to be nothrow
2062+
@goto delay_effects_analysis
2063+
end
2064+
end
20482065
# If there is no particular `@inbounds` for this function, then we only taint `:noinbounds`,
20492066
# which will subsequently taint `:consistent`-cy if this function is called from another
20502067
# function that uses `@inbounds`. However, if this `:boundscheck` is itself within an
@@ -2053,6 +2070,7 @@ function abstract_eval_value_expr(interp::AbstractInterpreter, e::Expr, vtypes::
20532070
merge_effects!(interp, sv, Effects(EFFECTS_TOTAL; noinbounds=false,
20542071
consistent = (get_curr_ssaflag(sv) & IR_FLAG_INBOUNDS) != 0 ? ALWAYS_FALSE : ALWAYS_TRUE))
20552072
end
2073+
@label delay_effects_analysis
20562074
rt = Bool
20572075
elseif head === :inbounds
20582076
@assert false && "Expected this to have been moved into flags"

test/compiler/effects.jl

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -683,11 +683,16 @@ end
683683

684684
# Test that dead `@inbounds` does not taint consistency
685685
# https://github.com/JuliaLang/julia/issues/48243
686-
@test Base.infer_effects() do
687-
false && @inbounds (1,2,3)[1]
686+
@test Base.infer_effects(Tuple{Int64}) do i
687+
false && @inbounds (1,2,3)[i]
688688
return 1
689689
end |> Core.Compiler.is_total
690690

691691
@test Base.infer_effects(Tuple{Int64}) do i
692692
@inbounds (1,2,3)[i]
693693
end |> !Core.Compiler.is_consistent
694+
695+
@test Base.infer_effects(Tuple{Tuple{Int64}}) do x
696+
@inbounds x[1]
697+
end |> Core.Compiler.is_foldable_nothrow
698+

0 commit comments

Comments
 (0)