Skip to content

Commit 5db930e

Browse files
authored
optimize: Handle path-excluded Core.ifelse arguments (JuliaLang#50312)
It's possible for PiNodes to effectively imply statically the condition of a Core.ifelse. For example: ```julia 23 ─ %60 = Core.ifelse(%47, false, true)::Bool │ %61 = Core.ifelse(%47, %58, false)::Union{Missing, Bool} 25 ─ goto #27 if not %60 26 ─ %65 = π (%61, Bool) └─── ... ``` In basic block #26, the PiNode gives us enough information to conclude that `%47 === false` if control flow reaches that point. The previous code incorrectly assumed that this kind of pruning would only be done for PhiNodes. Resolves JuliaLang#50276
1 parent 6d400e4 commit 5db930e

File tree

2 files changed

+67
-1
lines changed

2 files changed

+67
-1
lines changed

base/compiler/ssair/passes.jl

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -730,7 +730,7 @@ function perform_lifting!(compact::IncrementalCompact,
730730
new_node = Expr(:call, ifelse_func, condition) # Renamed then_result, else_result added below
731731
new_inst = NewInstruction(new_node, result_t, NoCallInfo(), old_inst[:line], old_inst[:flag])
732732

733-
ssa = insert_node!(compact, old_ssa, new_inst)
733+
ssa = insert_node!(compact, old_ssa, new_inst, #= attach_after =# true)
734734
lifted_philikes[i] = LiftedPhilike(ssa, IfElseCall(new_node), true)
735735
end
736736
# lifting_cache[ckey] = ssa
@@ -767,6 +767,21 @@ function perform_lifting!(compact::IncrementalCompact,
767767
else_result = lifted_value(compact, old_node_ssa, else_result,
768768
lifted_philikes, lifted_leaves, reverse_mapping)
769769

770+
# In cases where the Core.ifelse condition is statically-known, e.g., thanks
771+
# to a PiNode from a guarding conditional, replace with the remaining branch.
772+
if then_result === SKIP_TOKEN || else_result === SKIP_TOKEN
773+
only_result = (then_result === SKIP_TOKEN) ? else_result : then_result
774+
775+
# Replace Core.ifelse(%cond, %a, %b) with %a
776+
compact[lf.ssa][:inst] = only_result
777+
should_count && _count_added_node!(compact, only_result)
778+
779+
# Note: Core.ifelse(%cond, %a, %b) has observable effects (!nothrow), but since
780+
# we have not deleted the preceding statement that this was derived from, this
781+
# replacement is safe, i.e. it will not affect the effects observed.
782+
continue
783+
end
784+
770785
@assert then_result !== SKIP_TOKEN && then_result !== UNDEF_TOKEN
771786
@assert else_result !== SKIP_TOKEN && else_result !== UNDEF_TOKEN
772787

test/compiler/irpasses.jl

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -742,6 +742,57 @@ let m = Meta.@lower 1 + 1
742742
@test Core.Compiler.verify_ir(ir) === nothing
743743
end
744744

745+
# A lifted Core.ifelse with an eliminated branch (#50276)
746+
let m = Meta.@lower 1 + 1
747+
@assert Meta.isexpr(m, :thunk)
748+
src = m.args[1]::CodeInfo
749+
src.code = Any[
750+
# block 1
751+
#= %1: =# Core.Argument(2),
752+
# block 2
753+
#= %2: =# Expr(:call, Core.ifelse, SSAValue(1), true, missing),
754+
#= %3: =# GotoIfNot(SSAValue(2), 11),
755+
# block 3
756+
#= %4: =# PiNode(SSAValue(2), Bool), # <-- This PiNode is the trigger of the bug, since it
757+
# means that only one branch of the Core.ifelse
758+
# is lifted.
759+
#= %5: =# GotoIfNot(false, 8),
760+
# block 2
761+
#= %6: =# nothing,
762+
#= %7: =# GotoNode(8),
763+
# block 4
764+
#= %8: =# PhiNode(Int32[5, 7], Any[SSAValue(4), SSAValue(6)]),
765+
# ^-- N.B. This PhiNode also needs to have a Union{ ... } type in order
766+
# for lifting to be performed (it is skipped for e.g. `Bool`)
767+
#
768+
#= %9: =# Expr(:call, isa, SSAValue(8), Missing),
769+
#= %10: =# ReturnNode(SSAValue(9)),
770+
# block 5
771+
#= %11: =# ReturnNode(false),
772+
]
773+
src.ssavaluetypes = Any[
774+
Any,
775+
Union{Missing, Bool},
776+
Any,
777+
Bool,
778+
Any,
779+
Missing,
780+
Any,
781+
Union{Nothing, Bool},
782+
Bool,
783+
Any,
784+
Any
785+
]
786+
nstmts = length(src.code)
787+
src.codelocs = fill(one(Int32), nstmts)
788+
src.ssaflags = fill(one(Int32), nstmts)
789+
src.slotflags = fill(zero(UInt8), 3)
790+
ir = Core.Compiler.inflate_ir(src)
791+
@test Core.Compiler.verify_ir(ir) === nothing
792+
ir = @test_nowarn Core.Compiler.sroa_pass!(ir)
793+
@test Core.Compiler.verify_ir(ir) === nothing
794+
end
795+
745796
# Issue #31546 - missing widenconst in SROA
746797
function f_31546(x)
747798
(a, b) = x == "r" ? (false, false) :

0 commit comments

Comments
 (0)