Skip to content

Commit

Permalink
Merge pull request #43797 from ianatol/ia/condfolding
Browse files Browse the repository at this point in the history
Allow branch folding to look at type information, fix Conditional bug
  • Loading branch information
aviatesk committed Jan 15, 2022
2 parents 7b1cc4b + 35a5172 commit 7ab26e0
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 6 deletions.
11 changes: 6 additions & 5 deletions base/compiler/optimize.jl
Original file line number Diff line number Diff line change
Expand Up @@ -402,7 +402,8 @@ function finish(interp::AbstractInterpreter, opt::OptimizationState,
force_noinline = _any(@nospecialize(x) -> isexpr(x, :meta) && x.args[1] === :noinline, ir.meta)

# compute inlining and other related optimizations
if (isa(result, Const) || isconstType(result))
wresult = isa(result, InterConditional) ? widenconditional(result) : result
if (isa(wresult, Const) || isconstType(wresult))
proven_pure = false
# must be proven pure to use constant calling convention;
# otherwise we might skip throwing errors (issue #20704)
Expand Down Expand Up @@ -436,14 +437,14 @@ function finish(interp::AbstractInterpreter, opt::OptimizationState,
# Still set pure flag to make sure `inference` tests pass
# and to possibly enable more optimization in the future
src.pure = true
if isa(result, Const)
val = result.val
if isa(wresult, Const)
val = wresult.val
if is_inlineable_constant(val)
analyzed = ConstAPI(val)
end
else
@assert isconstType(result)
analyzed = ConstAPI(result.parameters[1])
@assert isconstType(wresult)
analyzed = ConstAPI(wresult.parameters[1])
end
force_noinline || (src.inlineable = true)
end
Expand Down
9 changes: 8 additions & 1 deletion base/compiler/ssair/ir.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1007,7 +1007,13 @@ function process_node!(compact::IncrementalCompact, result_idx::Int, inst::Instr
stmt = renumber_ssa2!(stmt, ssa_rename, used_ssas, late_fixup, result_idx, do_rename_ssa)::GotoIfNot
result[result_idx][:inst] = stmt
cond = stmt.cond
if isa(cond, Bool) && compact.fold_constant_branches
if compact.fold_constant_branches
if !isa(cond, Bool)
condT = widenconditional(argextype(cond, compact))
isa(condT, Const) || @goto bail
cond = condT.val
isa(cond, Bool) || @goto bail
end
if cond
result[result_idx][:inst] = nothing
kill_edge!(compact, active_bb, active_bb, stmt.dest)
Expand All @@ -1018,6 +1024,7 @@ function process_node!(compact::IncrementalCompact, result_idx::Int, inst::Instr
result_idx += 1
end
else
@label bail
result[result_idx][:inst] = GotoIfNot(cond, compact.bb_rename_succ[stmt.dest])
result_idx += 1
end
Expand Down
7 changes: 7 additions & 0 deletions test/compiler/inline.jl
Original file line number Diff line number Diff line change
Expand Up @@ -884,3 +884,10 @@ end
# have_fma elimination inside ^
f_pow() = ^(2.0, -1.0)
@test fully_eliminated(f_pow, Tuple{})

# bug where Conditional wasn't being properly marked as ConstAPI
let
@noinline fcond(a, b) = a === b
ftest(a) = (fcond(a, nothing); a)
@test fully_eliminated(ftest, Tuple{Bool})
end
15 changes: 15 additions & 0 deletions test/compiler/irpasses.jl
Original file line number Diff line number Diff line change
Expand Up @@ -758,3 +758,18 @@ let # effect-freeness computation for array allocation
nothing
end
end

# allow branch folding to look at type information
let ci = code_typed1(optimize=false) do
cond = 1 + 1 == 2
if !cond
gcd(24, 36)
else
gcd(64, 128)
end
end
ir = Core.Compiler.inflate_ir(ci)
@test count(@nospecialize(stmt)->isa(stmt, Core.GotoIfNot), ir.stmts.inst) == 1
ir = Core.Compiler.compact!(ir, true)
@test count(@nospecialize(stmt)->isa(stmt, Core.GotoIfNot), ir.stmts.inst) == 0
end

2 comments on commit 7ab26e0

@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(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 package evaluation job has completed - possible new issues were detected. A full report can be found here.

Please sign in to comment.