Skip to content

Commit

Permalink
Merge pull request #27265 from JuliaLang/kf/noglobalrefphi
Browse files Browse the repository at this point in the history
Don't accidentally put GlobalRefs into a PhiNode
  • Loading branch information
Keno committed May 27, 2018
2 parents dbefa09 + 13d9d9e commit b7ac84a
Show file tree
Hide file tree
Showing 7 changed files with 65 additions and 16 deletions.
28 changes: 24 additions & 4 deletions base/compiler/ssair/inlining2.jl
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,13 @@ function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector
stmt′ = ssa_substitute!(idx′, stmt′, argexprs, item.method.sig, item.sparams, linetable_offset, boundscheck_idx, compact)
if isa(stmt′, ReturnNode)
isa(stmt′.val, SSAValue) && (compact.used_ssas[stmt′.val.id] += 1)
return_value = stmt′.val
stmt′ = nothing
return_value = SSAValue(idx′)
inline_compact[idx′] = stmt′.val
val = stmt′.val
inline_compact.result_types[idx′] = (isa(val, Argument) || isa(val, Expr)) ?
compact_exprtype(compact, stmt′.val) :
compact_exprtype(inline_compact, stmt′.val)
break
end
inline_compact[idx′] = stmt′
end
Expand All @@ -313,9 +318,24 @@ function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector
stmt′ = ssa_substitute!(idx′, stmt′, argexprs, item.method.sig, item.sparams, linetable_offset, boundscheck_idx, compact)
if isa(stmt′, ReturnNode)
if isdefined(stmt′, :val)
val = stmt′.val
# GlobalRefs can have side effects, but are currently
# allowed in arguments of ReturnNodes
push!(pn.edges, inline_compact.active_result_bb-1)
push!(pn.values, stmt′.val)
stmt′ = GotoNode(post_bb_id)
if isa(val, GlobalRef) || isa(val, Expr)
stmt′ = val
inline_compact.result_types[idx′] = (isa(val, Argument) || isa(val, Expr)) ?
compact_exprtype(compact, val) :
compact_exprtype(inline_compact, val)
insert_node_here!(inline_compact, GotoNode(post_bb_id),
Any, compact.result_lines[idx′],
true)
push!(pn.values, SSAValue(idx′))
else
push!(pn.values, val)
stmt′ = GotoNode(post_bb_id)
end

end
elseif isa(stmt′, GotoNode)
stmt′ = GotoNode(stmt′.label + bb_offset)
Expand Down
22 changes: 14 additions & 8 deletions base/compiler/ssair/ir.jl
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ struct CFG
blocks::Vector{BasicBlock}
index::Vector{Int}
end
copy(c::CFG) = CFG(copy(c.blocks), copy(c.index))

function block_for_inst(index, inst)
searchsortedfirst(index, inst, lt=(<=))
Expand Down Expand Up @@ -156,6 +157,7 @@ struct NewNode
# The index into the line number table of this entry
line::Int
end
copy(n::NewNode) = copy(n.pos, n.attach_after, n.typ, copy(n.node), n.line)

struct IRCode
stmts::Vector{Any}
Expand All @@ -178,6 +180,8 @@ struct IRCode
return new(stmts, types, lines, flags, ir.argtypes, ir.linetable, cfg, new_nodes, ir.mod, ir.meta)
end
end
copy(code::IRCode) = IRCode(code, copy(code.stmts), copy(code.types),
copy(code.lines), copy(code.flags), copy(code.cfg), copy(code.new_nodes))

function getindex(x::IRCode, s::SSAValue)
if s.id <= length(x.stmts)
Expand Down Expand Up @@ -423,6 +427,7 @@ mutable struct IncrementalCompact
idx::Int
result_idx::Int
active_result_bb::Int
renamed_new_nodes::Bool
function IncrementalCompact(code::IRCode)
# Sort by position with attach after nodes affter regular ones
perm = my_sortperm(Int[(code.new_nodes[i].pos*2 + Int(code.new_nodes[i].attach_after)) for i in 1:length(code.new_nodes)])
Expand All @@ -439,7 +444,7 @@ mutable struct IncrementalCompact
pending_perm = Int[]
return new(code, result, result_types, result_lines, result_flags, code.cfg.blocks, ssa_rename, used_ssas, late_fixup, perm, 1,
new_new_nodes, pending_nodes, pending_perm,
1, 1, 1)
1, 1, 1, false)
end

# For inlining
Expand All @@ -456,7 +461,7 @@ mutable struct IncrementalCompact
parent.result_bbs, ssa_rename, parent.used_ssas,
late_fixup, perm, 1,
new_new_nodes, pending_nodes, pending_perm,
1, result_offset, parent.active_result_bb)
1, result_offset, parent.active_result_bb, false)
end
end

Expand Down Expand Up @@ -624,19 +629,19 @@ function getindex(view::TypesView, idx)
isa(idx, SSAValue) && (idx = idx.id)
if isa(view.ir, IncrementalCompact) && idx < view.ir.result_idx
return view.ir.result_types[idx]
elseif isa(view.ir, IncrementalCompact) && view.ir.renamed_new_nodes
if idx <= length(view.ir.result_types)
return view.ir.result_types[idx]
else
return view.ir.new_new_nodes[idx - length(view.ir.result_types)].typ
end
else
ir = isa(view.ir, IncrementalCompact) ? view.ir.ir : view.ir
if idx <= length(ir.types)
return ir.types[idx]
else
return ir.new_nodes[idx - length(ir.types)].typ
end
ir = ir.ir
end
if idx <= length(ir.types)
return ir.types[idx]
else
return ir.new_nodes[idx - length(ir.types)].typ
end
end

Expand Down Expand Up @@ -955,6 +960,7 @@ function non_dce_finish!(compact::IncrementalCompact)
bb = compact.result_bbs[end]
compact.result_bbs[end] = BasicBlock(bb,
StmtRange(first(bb.stmts), result_idx-1))
compact.renamed_new_nodes = true
end

function finish(compact::IncrementalCompact)
Expand Down
10 changes: 10 additions & 0 deletions base/compiler/ssair/passes.jl
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,11 @@ function lift_leaves(compact::IncrementalCompact, @nospecialize(stmt),
if isa(leaf, OldSSAValue) && isa(lifted, SSAValue)
lifted = OldSSAValue(lifted.id)
end
if isa(lifted, GlobalRef) || isa(lifted, Expr)
lifted = insert_node!(compact, leaf, compact_exprtype(compact, lifted), lifted)
def.args[1+field] = lifted
(isa(leaf, SSAValue) && (leaf.id < compact.result_idx)) && push!(compact.late_fixup, leaf.id)
end
lifted_leaves[leaf_key] = RefValue{Any}(lifted)
continue
elseif isexpr(def, :new)
Expand Down Expand Up @@ -293,6 +298,11 @@ function lift_leaves(compact::IncrementalCompact, @nospecialize(stmt),
if isa(leaf, OldSSAValue) && isa(lifted, SSAValue)
lifted = OldSSAValue(lifted.id)
end
if isa(lifted, GlobalRef) || isa(lifted, Expr)
lifted = insert_node!(compact, leaf, compact_exprtype(compact, lifted), lifted)
def.args[1+field] = lifted
(isa(leaf, SSAValue) && (leaf.id < compact.result_idx)) && push!(compact.late_fixup, leaf.id)
end
lifted_leaves[leaf_key] = RefValue{Any}(lifted)
continue
else
Expand Down
2 changes: 1 addition & 1 deletion base/compiler/ssair/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,7 @@ function show_ir(io::IO, code::IRCode, expr_type_printer=default_expr_type_print
print_sep = true
end
floop = true
while !isempty(new_nodes_perm) && new_nodes[peek(new_nodes_perm)].pos == idx
while !isempty(new_nodes_perm) && new_nodes[Iterators.peek(new_nodes_perm)].pos == idx
node_idx = popfirst!(new_nodes_perm)
new_node = new_nodes[node_idx]
node_idx += length(code.stmts)
Expand Down
3 changes: 3 additions & 0 deletions base/compiler/ssair/verify.jl
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ function verify_ir(ir::IRCode)
#"""
#error()
end
elseif isa(val, GlobalRef) || isa(val, Expr)
@verify_error "GlobalRefs and Exprs are not allowed as PhiNode values"
error()
end
check_op(ir, domtree, val, edge, last(ir.cfg.blocks[stmt.edges[i]].stmts)+1)
end
Expand Down
11 changes: 11 additions & 0 deletions test/core.jl
Original file line number Diff line number Diff line change
Expand Up @@ -6138,3 +6138,14 @@ foo27204(x) = f27204(x)()
end
g27209(x) = f27209(x ? nothing : 1.0)
@test g27209(true) == true

# Issue 27240
@inline function foo27240()
if rand(Bool)
return foo_nonexistant_27240
else
return bar_nonexistant_27240
end
end
bar27240() = foo27240()
@test_throws UndefVarError bar27240()
5 changes: 2 additions & 3 deletions test/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1236,8 +1236,7 @@ function f_line()
nothing
end
h_line() = f_line()
@test compute_annotations(h_line, Tuple{}) == """
@test startswith(compute_annotations(h_line, Tuple{}), """
│╻╷ f_line
││╻ g_line
││╻ g_line
"""
││╻ g_line""")

0 comments on commit b7ac84a

Please sign in to comment.