Use correct effect_free predicate in inlining

As part of the new optimizer work, I wrote a more accurate effect_free
predicate (stmt_effect_free). However, the old optimizer's less accurate
predicate stuck around and snuck into two places
 1) The inliner
 2) The optimize.jl code that computes proven_pure

As a result, the compiler would eliminate certain statements, even if it could
not prove that they were effect_free. Fix all that by getting rid
of the old predicate and using the new one everywhere.

Fixes #27910

base/compiler/optimize.jl

@@ -144,7 +144,6 @@ function optimize(opt::OptimizationState, @nospecialize(result))
  nargs = Int(opt.nargs) - 1
  @timeit "optimizer" ir = run_passes(opt.src, nargs, opt)
  force_noinline = _any(@nospecialize(x) -> isexpr(x, :meta) && x.args[1] == :noinline, ir.meta)
- replace_code_newstyle!(opt.src, ir, nargs)
 
  # compute inlining and other related optimizations
  if (isa(result, Const) || isconstType(result))
@@ -153,10 +152,17 @@ function optimize(opt::OptimizationState, @nospecialize(result))
  # (issue #20704)
  # TODO: Improve this analysis; if a function is marked @pure we should really
  # only care about certain errors (e.g. method errors and type errors).
- if length(opt.src.code) < 10
+ if length(ir.stmts) < 10
  proven_pure = true
- for i in 1:length(opt.src.code)
- if !statement_effect_free(opt.src.code[i], opt, opt.src.ssavaluetypes[i])
+ for i in 1:length(ir.stmts)
+ stmt = ir.stmts[i]
+ # These affect control flow within the function (so may not be removed
+ # if there is no usage within the function), but don't affect the purity
+ # of the function as a whole.
+ if isa(stmt, GotoIfNot) || isa(stmt, GotoNode) || isa(stmt, ReturnNode)
+ continue
+ end
+ if !stmt_effect_free(stmt, ir, ir.spvals)
  proven_pure = false
  break
  end
@@ -188,6 +194,8 @@ function optimize(opt::OptimizationState, @nospecialize(result))
  end
  end
 
+ replace_code_newstyle!(opt.src, ir, nargs)
+
  # determine and cache inlineability
  if !force_noinline
  # don't keep ASTs for functions specialized on a Union argument
@@ -252,118 +260,6 @@ function is_pure_builtin(@nospecialize(f))
  end
 end
 
-function statement_effect_free(@nospecialize(e), me::OptimizationState, @nospecialize(etype))
- if isa(e, Expr)
- if e.head === :(=)
- return !isa(e.args[1], GlobalRef) && effect_free(e.args[2], me, false, etype)
- elseif e.head === :gotoifnot
- return effect_free(e.args[1], me, false, etype)
- end
- elseif isa(e, GotoNode)
- return true
- end
- return effect_free(e, me, false, etype)
-end
-
-effect_free(@nospecialize(e), s::OptimizationState, allow_volatile::Bool, @nospecialize(etype)) =
- effect_free(e, s.src, s.sp, allow_volatile, etype, s.slottypes)
-
-# detect some important side-effect-free calls (allow_volatile=true)
-# and some affect-free calls (allow_volatile=false) -- affect_free means the call
-# cannot be affected by previous calls, except assignment nodes
-function effect_free(@nospecialize(e), src, spvals::SimpleVector, allow_volatile::Bool, @nospecialize(etype),
- slottypes::Vector{Any} = empty_slottypes)
- if isa(e, GlobalRef)
- return (isdefined(e.mod, e.name) && (allow_volatile || isconst(e.mod, e.name)))
- elseif isa(e, Slot)
- return src.slotflags[slot_id(e)] & SLOT_USEDUNDEF == 0
- elseif isa(e, Expr)
- e = e::Expr
- head = e.head
- if is_meta_expr_head(head)
- return true
- end
- if head === :static_parameter
- # if we aren't certain enough about the type, it might be an UndefVarError at runtime
- return isa(etype, Const) || issingletontype(widenconst(etype))
- end
- if etype === Bottom
- return false
- end
- ea = e.args
- if head === :call
- if is_known_call_p(e, is_pure_builtin, src, spvals, slottypes)
- if !allow_volatile
- if is_known_call(e, arrayref, src, spvals, slottypes) || is_known_call(e, arraylen, src, spvals, slottypes)
- return false
- elseif is_known_call(e, getfield, src, spvals, slottypes)
- nargs = length(ea)
- (3 <= nargs <= 4) || return false
- # TODO: check ninitialized
- if !isa(etype, Const) && !isconstType(etype)
- # first argument must be immutable to ensure e is affect_free
- a = ea[2]
- typ = unwrap_unionall(widenconst(argextype(a, src, spvals, slottypes)))
- if isType(typ)
- # all fields of subtypes of Type are effect-free
- # (including the non-inferrable uid field)
- elseif !isa(typ, DataType) || typ.abstract || (typ.mutable && length(typ.types) > 0)
- return false
- end
- end
- end
- end
- # fall-through
- elseif is_known_call(e, _apply, src, spvals, slottypes) && length(ea) > 1
- ft = argextype(ea[2], src, spvals, slottypes)
- if !isa(ft, Const) || (!contains_is(_PURE_BUILTINS, ft.val) &&
- ft.val !== Core.sizeof)
- return false
- end
- # fall-through
- else
- return false
- end
- elseif head === :new
- a = ea[1]
- typ = argextype(a, src, spvals, slottypes)
- # `Expr(:new)` of unknown type could raise arbitrary TypeError.
- typ, isexact = instanceof_tfunc(typ)
- isexact || return false
- isconcretedispatch(typ) || return false
- typ = typ::DataType
- if !allow_volatile && typ.mutable
- return false
- end
- fieldcount(typ) >= length(ea) - 1 || return false
- for fld_idx in 1:(length(ea) - 1)
- eT = argextype(ea[fld_idx + 1], src, spvals, slottypes)
- fT = fieldtype(typ, fld_idx)
- eT ⊑ fT || return false
- end
- # fall-through
- elseif head === :return
- # fall-through
- elseif head === :isdefined
- return allow_volatile
- elseif head === :the_exception
- return allow_volatile
- elseif head === :copyast
- return true
- else
- return false
- end
- for a in ea
- if !effect_free(a, src, spvals, allow_volatile, argextype(a, src, spvals, slottypes))
- return false
- end
- end
- elseif isa(e, GotoNode)
- return false
- end
- return true
-end
-
 ## Computing the cost of a function body
 
 # saturating sum (inputs are nonnegative), prevents overflow with typemax(Int) below

base/compiler/ssair/inlining.jl

@@ -1027,7 +1027,7 @@ function early_inline_special_case(ir::IRCode, @nospecialize(f), @nospecialize(f
  (f === Core.kwfunc && length(atypes) == 2) ||
  (is_inlineable_constant(val) &&
  (contains_is(_PURE_BUILTINS, f) ||
- (f === getfield && effect_free(e, ir, ir.spvals, false, etype)) ||
+ (f === getfield && stmt_effect_free(e, ir, ir.spvals)) ||
  (isa(f, IntrinsicFunction) && is_pure_intrinsic_optim(f)))))
  return quoted(val)
  end

base/compiler/ssair/queries.jl

@@ -3,7 +3,7 @@
 """
 Determine whether a statement is side-effect-free, i.e. may be removed if it has no uses.
 """
-function stmt_effect_free(@nospecialize(stmt), src::IncrementalCompact, spvals::SimpleVector)
+function stmt_effect_free(@nospecialize(stmt), src, spvals::SimpleVector)
  isa(stmt, Union{PiNode, PhiNode}) && return true
  isa(stmt, Union{ReturnNode, GotoNode, GotoIfNot}) && return false
  isa(stmt, GlobalRef) && return isdefined(stmt.mod, stmt.name)

test/core.jl

@@ -6270,3 +6270,7 @@ foo27770() = get27770(Nullable27770(), Handle27770())
 
 bar27770() = Nullable27770().value
 @test_throws UndefRefError bar27770()
+
+# Issue 27910
+f27910() = ((),)[2]
+@test_throws BoundsError f27910()