Skip to content

Commit

Permalink
a minor improvement for EA-based :effect_free-ness refinement (#55796)
Browse files Browse the repository at this point in the history
  • Loading branch information
aviatesk authored Sep 19, 2024
1 parent 86c567f commit 58b239c
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 3 deletions.
6 changes: 4 additions & 2 deletions base/compiler/optimize.jl
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,8 @@ function check_all_args_noescape!(sv::PostOptAnalysisState, ir::IRCode, @nospeci
else
return false
end
has_no_escape(x::EscapeAnalysis.EscapeInfo) =
EscapeAnalysis.has_no_escape(EscapeAnalysis.ignore_argescape(x))
for i = startidx:length(stmt.args)
arg = stmt.args[i]
argt = argextype(arg, ir)
Expand All @@ -710,7 +712,7 @@ function check_all_args_noescape!(sv::PostOptAnalysisState, ir::IRCode, @nospeci
end
# See if we can find the allocation
if isa(arg, Argument)
if EscapeAnalysis.has_no_escape(EscapeAnalysis.ignore_argescape(estate[arg]))
if has_no_escape(estate[arg])
# Even if we prove everything else effect_free, the best we can
# say is :effect_free_if_argmem_only
if sv.effect_free_if_argmem_only === nothing
Expand All @@ -721,7 +723,7 @@ function check_all_args_noescape!(sv::PostOptAnalysisState, ir::IRCode, @nospeci
end
return false
elseif isa(arg, SSAValue)
EscapeAnalysis.has_no_escape(estate[arg]) || return false
has_no_escape(estate[arg]) || return false
check_all_args_noescape!(sv, ir, ir[arg][:stmt], estate) || return false
else
return false
Expand Down
2 changes: 1 addition & 1 deletion base/compiler/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ getresult(info::CallInfo, idx::Int) = getresult_impl(info, idx)
# must implement `nsplit`, `getsplit`, and `add_uncovered_edges!` to opt in to inlining
nsplit_impl(::CallInfo) = nothing
getsplit_impl(::CallInfo, ::Int) = error("unexpected call into `getsplit`")
add_uncovered_edges_impl(edges::Vector{Any}, info::CallInfo, @nospecialize(atype)) = error("unexpected call into `add_uncovered_edges!`")
add_uncovered_edges_impl(::Vector{Any}, ::CallInfo, _) = error("unexpected call into `add_uncovered_edges!`")

# must implement `getresult` to opt in to extended lattice return information
getresult_impl(::CallInfo, ::Int) = nothing
Expand Down
8 changes: 8 additions & 0 deletions test/compiler/effects.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1141,6 +1141,14 @@ end
@test_broken Core.Compiler.is_effect_free(Base.infer_effects(set_arr_with_unused_arg_2, (Vector{Int},)))
@test_broken Core.Compiler.is_effect_free_if_inaccessiblememonly(Base.infer_effects(set_arg_arr!, (Vector{Int},)))

# EA-based refinement of :effect_free
function f_EA_refine(ax, b)
bx = Ref{Any}()
@noinline bx[] = b
return ax[] + b
end
@test Core.Compiler.is_effect_free(Base.infer_effects(f_EA_refine, (Base.RefValue{Int},Int)))

function issue51837(; openquotechar::Char, newlinechar::Char)
ncodeunits(openquotechar) == 1 || throw(ArgumentError("`openquotechar` must be a single-byte character"))
if !isnothing(newlinechar)
Expand Down

5 comments on commit 58b239c

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

@vtjnash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

@vtjnash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like something incurred a penalty on inference time, but possibly also a boost to skipmissing?

["inference", "abstract interpretation", "many_global_refs"] 1.87 (5%) ❌ 1.12

Please sign in to comment.