Skip to content

Commit

Permalink
Delay :boundscheck resolution until codegen time
Browse files Browse the repository at this point in the history
  • Loading branch information
Keno committed Aug 21, 2023
1 parent 4238f18 commit 96b55e0
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 16 deletions.
26 changes: 17 additions & 9 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,19 @@ function ir_prepare_inlining!(insert_node!::Inserter, inline_target::Union{IRCod
return SSASubstitute(mi, argexprs, spvals_ssa, linetable_offset)
end

function adjust_boundscheck!(inline_compact, idx′, stmt, boundscheck)
if boundscheck === :off
if length(stmt.args) == 0
inline_compact[SSAValue(idx′)][:flag] |= IR_FLAG_INBOUNDS
end
elseif boundscheck !== :propagate
if (inline_compact[SSAValue(idx′)][:flag] & IR_FLAG_INBOUNDS) == 0
# Prevent future inlining passes from setting IR_FLAG_INBOUNDS
length(stmt.args) == 0 && push!(stmt.args, true)
end
end
end

function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector{Any},
item::InliningTodo, boundscheck::Symbol, todo_bbs::Vector{Tuple{Int, Int}})
# Ok, do the inlining here
Expand Down Expand Up @@ -424,6 +437,8 @@ function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector
# Everything legal in value position is guaranteed to be effect free in stmt position
inline_compact.result[idx′][:flag] = IR_FLAG_EFFECT_FREE | IR_FLAG_NOTHROW
break
elseif isexpr(stmt′, :boundscheck)
adjust_boundscheck!(inline_compact, idx′, stmt′, boundscheck)
end
inline_compact[idx′] = stmt′
end
Expand Down Expand Up @@ -460,6 +475,8 @@ function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector
stmt′ = GotoIfNot(stmt′.cond, stmt′.dest + bb_offset)
elseif isa(stmt′, PhiNode)
stmt′ = PhiNode(Int32[edge+bb_offset for edge in stmt′.edges], stmt′.values)
elseif isexpr(stmt′, :boundscheck)
adjust_boundscheck!(inline_compact, idx′, stmt′, boundscheck)
end
inline_compact[idx′] = stmt′
end
Expand Down Expand Up @@ -1804,7 +1821,6 @@ end

function ssa_substitute!(insert_node!::Inserter, subst_inst::Instruction, @nospecialize(val),
ssa_substitute::SSASubstitute, boundscheck::Symbol)
subst_inst[:flag] &= ~IR_FLAG_INBOUNDS
subst_inst[:line] += ssa_substitute.linetable_offset
return ssa_substitute_op!(insert_node!, subst_inst, val, ssa_substitute, boundscheck)
end
Expand Down Expand Up @@ -1874,14 +1890,6 @@ function ssa_substitute_op!(insert_node!::Inserter, subst_inst::Instruction, @no
for argt in e.args[3]::SimpleVector ]...)
end
end
elseif head === :boundscheck
if boundscheck === :off # inbounds == true
return false
elseif boundscheck === :propagate
return e
else # on or default
return true
end
end
end
isa(val, Union{SSAValue, NewSSAValue}) && return val # avoid infinite loop
Expand Down
2 changes: 1 addition & 1 deletion base/compiler/ssair/verify.jl
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ if !isdefined(@__MODULE__, Symbol("@verify_error"))
end
end

is_value_pos_expr_head(head::Symbol) = head === :boundscheck
is_value_pos_expr_head(head::Symbol) = false
function check_op(ir::IRCode, domtree::DomTree, @nospecialize(op), use_bb::Int, use_idx::Int, printed_use_idx::Int, print::Bool, isforeigncall::Bool, arg_idx::Int, allow_frontend_forms::Bool)
if isa(op, SSAValue)
if op.id > length(ir.stmts)
Expand Down
2 changes: 1 addition & 1 deletion base/compiler/validation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ is_valid_lvalue(@nospecialize(x)) = isa(x, UnoptSlot) || isa(x, GlobalRef)

function is_valid_argument(@nospecialize(x))
if isa(x, UnoptSlot) || isa(x, Argument) || isa(x, SSAValue) ||
isa(x, GlobalRef) || isa(x, QuoteNode) || isexpr(x, (:static_parameter, :boundscheck)) ||
isa(x, GlobalRef) || isa(x, QuoteNode) || is_value_pos_expr_head(x) ||
isa(x, Number) || isa(x, AbstractString) || isa(x, AbstractChar) || isa(x, Tuple) ||
isa(x, Type) || isa(x, Core.Box) || isa(x, Module) || x === nothing
return true
Expand Down
3 changes: 3 additions & 0 deletions src/codegen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5146,6 +5146,9 @@ static void emit_ssaval_assign(jl_codectx_t &ctx, ssize_t ssaidx_0based, jl_valu
it = ctx.phic_slots.emplace(ssaidx_0based, jl_varinfo_t(ctx.builder.getContext())).first;
}
slot = emit_varinfo(ctx, it->second, jl_symbol("phic"));
} else if (jl_is_expr(r) && ((jl_expr_t*)r)->head == jl_boundscheck_sym) {
uint8_t flag = jl_array_uint8_ref(ctx.source->ssaflags, ssaidx_0based);
slot = mark_julia_const(ctx, bounds_check_enabled(ctx, (flag & IR_FLAG_INBOUNDS) ? jl_false : jl_true) ? jl_true : jl_false);
} else {
slot = emit_expr(ctx, r, ssaidx_0based); // slot could be a jl_value_t (unboxed) or jl_value_t* (ispointer)
}
Expand Down
2 changes: 1 addition & 1 deletion src/julia-syntax.scm
Original file line number Diff line number Diff line change
Expand Up @@ -4256,7 +4256,7 @@ f(x) = yt(x)
(or (simple-atom? e) (symbol? e)
(and (pair? e)
(memq (car e) '(quote inert top core globalref outerref
slot static_parameter boundscheck)))))
slot static_parameter)))))

(define (valid-ir-rvalue? lhs e)
(or (ssavalue? lhs)
Expand Down
2 changes: 2 additions & 0 deletions src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -1653,6 +1653,8 @@ JL_DLLEXPORT uint32_t jl_crc32c(uint32_t crc, const char *buf, size_t len);

// -- exports from codegen -- //

#define IR_FLAG_INBOUNDS 0x01

JL_DLLIMPORT jl_code_instance_t *jl_generate_fptr(jl_method_instance_t *mi JL_PROPAGATES_ROOT, size_t world);
JL_DLLIMPORT void jl_generate_fptr_for_unspecialized(jl_code_instance_t *unspec);
JL_DLLIMPORT void jl_generate_fptr_for_oc_wrapper(jl_code_instance_t *unspec);
Expand Down
6 changes: 5 additions & 1 deletion src/method.c
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,10 @@ static void jl_code_info_set_ir(jl_code_info_t *li, jl_expr_t *ir)
}
bd[j] = jl_nothing;
}
else if (jl_is_expr(st) && ((jl_expr_t*)st)->head == jl_boundscheck_sym) {
// Don't set IR_FLAG_INBOUNDS on boundscheck at the same level
is_flag_stmt = 1;
}
else if (jl_is_expr(st) && ((jl_expr_t*)st)->head == jl_return_sym) {
jl_array_ptr_set(body, j, jl_new_struct(jl_returnnode_type, jl_exprarg(st, 0)));
}
Expand All @@ -392,7 +396,7 @@ static void jl_code_info_set_ir(jl_code_info_t *li, jl_expr_t *ir)
else {
uint8_t flag = 0;
if (inbounds_depth > 0)
flag |= 1 << 0;
flag |= IR_FLAG_INBOUNDS;
if (inline_flags->len > 0) {
void* inline_flag = inline_flags->items[inline_flags->len - 1];
flag |= 1 << (inline_flag ? 1 : 2);
Expand Down
5 changes: 2 additions & 3 deletions test/boundscheck_exec.jl
Original file line number Diff line number Diff line change
Expand Up @@ -252,10 +252,9 @@ end

# Boundschecking removal of indices with different type, see #40281
getindex_40281(v, a, b, c) = @inbounds getindex(v, a, b, c)
typed_40281 = sprint((io, args...) -> code_warntype(io, args...; optimize=true), getindex_40281, Tuple{Array{Float64, 3}, Int, UInt8, Int})
llvm_40281 = sprint((io, args...) -> code_llvm(io, args...; optimize=true), getindex_40281, Tuple{Array{Float64, 3}, Int, UInt8, Int})
if bc_opt == bc_default || bc_opt == bc_off
@test occursin("arrayref(false", typed_40281)
@test !occursin("arrayref(true", typed_40281)
@test !occursin("call void @ijl_bounds_error_ints", llvm_40281)
end

# Given this is a sub-processed test file, not using @testsets avoids
Expand Down

0 comments on commit 96b55e0

Please sign in to comment.