Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion base/compiler/ssair/passes.jl
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,7 @@ function perform_lifting!(compact::IncrementalCompact,
new_node = Expr(:call, ifelse_func, condition) # Renamed then_result, else_result added below
new_inst = NewInstruction(new_node, result_t, NoCallInfo(), old_inst[:line], old_inst[:flag])

ssa = insert_node!(compact, old_ssa, new_inst)
ssa = insert_node!(compact, old_ssa, new_inst, #= attach_after =# true)
lifted_philikes[i] = LiftedPhilike(ssa, IfElseCall(new_node), true)
end
# lifting_cache[ckey] = ssa
Expand Down Expand Up @@ -767,6 +767,21 @@ function perform_lifting!(compact::IncrementalCompact,
else_result = lifted_value(compact, old_node_ssa, else_result,
lifted_philikes, lifted_leaves, reverse_mapping)

# In cases where the Core.ifelse condition is statically-known, e.g., thanks
# to a PiNode from a guarding conditional, replace with the remaining branch.
if then_result === SKIP_TOKEN || else_result === SKIP_TOKEN
only_result = (then_result === SKIP_TOKEN) ? else_result : then_result

# Replace Core.ifelse(%cond, %a, %b) with %a
compact[lf.ssa][:inst] = only_result
Copy link
Member

Choose a reason for hiding this comment

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

On second look, this bypasses the count removal of the cond. This probably needs to be:

compact[lf.ssa] = nothing
compact[lf.ssa][:inst] = only_result

Possibly also consider turning on the oracle check for the tests, which would have caught this.

should_count && _count_added_node!(compact, only_result)

# Note: Core.ifelse(%cond, %a, %b) has observable effects (!nothrow), but since
# we have not deleted the preceding statement that this was derived from, this
# replacement is safe, i.e. it will not affect the effects observed.
continue
end

@assert then_result !== SKIP_TOKEN && then_result !== UNDEF_TOKEN
@assert else_result !== SKIP_TOKEN && else_result !== UNDEF_TOKEN

Expand Down
51 changes: 51 additions & 0 deletions test/compiler/irpasses.jl
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,57 @@ let m = Meta.@lower 1 + 1
@test Core.Compiler.verify_ir(ir) === nothing
end

# A lifted Core.ifelse with an eliminated branch (#50276)
let m = Meta.@lower 1 + 1
@assert Meta.isexpr(m, :thunk)
src = m.args[1]::CodeInfo
src.code = Any[
# block 1
#= %1: =# Core.Argument(2),
# block 2
#= %2: =# Expr(:call, Core.ifelse, SSAValue(1), true, missing),
#= %3: =# GotoIfNot(SSAValue(2), 11),
# block 3
#= %4: =# PiNode(SSAValue(2), Bool), # <-- This PiNode is the trigger of the bug, since it
# means that only one branch of the Core.ifelse
# is lifted.
#= %5: =# GotoIfNot(false, 8),
# block 2
#= %6: =# nothing,
#= %7: =# GotoNode(8),
# block 4
#= %8: =# PhiNode(Int32[5, 7], Any[SSAValue(4), SSAValue(6)]),
# ^-- N.B. This PhiNode also needs to have a Union{ ... } type in order
# for lifting to be performed (it is skipped for e.g. `Bool`)
#
#= %9: =# Expr(:call, isa, SSAValue(8), Missing),
#= %10: =# ReturnNode(SSAValue(9)),
# block 5
#= %11: =# ReturnNode(false),
Comment on lines +750 to +771
Copy link
Member

Choose a reason for hiding this comment

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

nice 👍

]
src.ssavaluetypes = Any[
Any,
Union{Missing, Bool},
Any,
Bool,
Any,
Missing,
Any,
Union{Nothing, Bool},
Bool,
Any,
Any
]
nstmts = length(src.code)
src.codelocs = fill(one(Int32), nstmts)
src.ssaflags = fill(one(Int32), nstmts)
src.slotflags = fill(zero(UInt8), 3)
ir = Core.Compiler.inflate_ir(src)
@test Core.Compiler.verify_ir(ir) === nothing
ir = @test_nowarn Core.Compiler.sroa_pass!(ir)
@test Core.Compiler.verify_ir(ir) === nothing
end

# Issue #31546 - missing widenconst in SROA
function f_31546(x)
(a, b) = x == "r" ? (false, false) :
Expand Down