Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow constant-folding intrinsics that are non-pure for inference only #31193

Closed
wants to merge 1 commit into from
Closed
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
12 changes: 8 additions & 4 deletions base/compiler/optimize.jl
Original file line number Diff line number Diff line change
Expand Up @@ -252,17 +252,21 @@ function optimize(opt::OptimizationState, @nospecialize(result))
nothing
end


# whether `f` is pure for inference
function is_pure_intrinsic_infer(f::IntrinsicFunction)
# whether `f` is pure for optimization purposes
function is_pure_intrinsic_optimize(f::IntrinsicFunction)
return !(f === Intrinsics.pointerref || # this one is volatile
f === Intrinsics.pointerset || # this one is never effect-free
f === Intrinsics.llvmcall || # this one is never effect-free
f === Intrinsics.arraylen || # this one is volatile
f === Intrinsics.sqrt_llvm || # this one may differ at runtime (by a few ulps)
f === Intrinsics.cglobal) # cglobal lookup answer changes at runtime
end

# whether `f` is pure for inference
is_pure_intrinsic_optimize_only(f) = f === Intrinsics.sqrt_llvm # this one may differ by a few ulps at runtime
Copy link
Member

Choose a reason for hiding this comment

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

This list should have all of the _fast intrinsics too plus muladd_float. I thought I had remembered to add those to the is_pure_intrinsic_infer list originally, but seems like somehow only sqrt_llvm actually made it on the list—which is arguably the least likely one to actually be a problem.

function is_pure_intrinsic_infer(f::IntrinsicFunction)
return is_pure_intrinsic_optimize(f) && !is_pure_intrinsic_optimize_only(f)
end

# whether `f` is effect free if nothrow
intrinsic_effect_free_if_nothrow(f) = f === Intrinsics.pointerref || is_pure_intrinsic_infer(f)

Expand Down
21 changes: 21 additions & 0 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1077,6 +1077,21 @@ function ispuretopfunction(@nospecialize(f))
istopfunction(f, :promote_type)
end

function intrinsic_tfunc_optimize(f::IntrinsicFunction, atypes::Vector{Any})
is_pure_intrinsic_optimize(f) || return nothing
args = atypes[2:end]
_all(@nospecialize(a) -> isa(a, Const), args) || return nothing
iidx = Int(reinterpret(Int32, f::IntrinsicFunction)) + 1
if iidx < 0 || iidx > length(T_IFUNC)
# invalid intrinsic
return nothing
end
(min, max, _) = T_IFUNC[iidx]
Copy link
Member

Choose a reason for hiding this comment

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

Seems like you're trying to do error checking here, then only get around to doing half of it? I think we can reuse most of pure_eval_call here, rather than implementing an arbitrary subset of it.

Copy link
Member Author

Choose a reason for hiding this comment

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

ah, oops that code got lost somewhere. I don't think pure_eval_call handles intrinsics though.

Copy link
Member

Choose a reason for hiding this comment

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

huh, that seems sort of a little bit silly. I didn't realize we had a third copy of that code sitting around too.

Copy link
Member Author

Choose a reason for hiding this comment

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

So should I add support for intrinsics to pure_eval_call then? I would kind of like to avoid the try/catch since we do have nothrow modeling for intrinsics.

Copy link
Member Author

Choose a reason for hiding this comment

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

In any case, I've just added the error check here for now.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it's probably OK to reuse that support, although a bit less general (can't handle some functions where it might throw, but computing that set is pretty tedious—e.g. sizeof_tfunc). But it doesn't look like you've got any calls to the no-throw predicate right now.

Copy link
Member

Choose a reason for hiding this comment

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

Seems to still be missing a call to the nothrow-test or a try/catch to handle it?

min <= length(atypes) - 1 <= max || return nothing
argvals = anymap(a::Const -> a.val, atypes[2:end])
return f(argvals...)
end

function early_inline_special_case(ir::IRCode, @nospecialize(f), @nospecialize(ft), e::Expr, atypes::Vector{Any}, sv::OptimizationState,
@nospecialize(etype))
if (f === typeassert || ft ⊑ typeof(typeassert)) && length(atypes) == 3
Expand Down Expand Up @@ -1106,6 +1121,12 @@ function early_inline_special_case(ir::IRCode, @nospecialize(f), @nospecialize(f
return quoted(val)
end
end
elseif isa(f, IntrinsicFunction) && is_pure_intrinsic_optimize_only(f)
# If this intrinsic was not eligible for constant propagation during
# inference, but is now, try to do that here.
val = intrinsic_tfunc_optimize(f, atypes)
val === nothing && return nothing
return quoted(val)
end
end

Expand Down
11 changes: 11 additions & 0 deletions test/compiler/inline.jl
Original file line number Diff line number Diff line change
Expand Up @@ -247,3 +247,14 @@ let code = code_typed(f_pointerref, Tuple{Type{Int}})[1][1].code
end
@test !any_ptrref
end

f_sqrt() = sqrt(2)
let code = code_typed(f_sqrt, Tuple{})[1][1].code
@test length(code) == 1
@test isa(code[1], Expr) && code[1].head == :return && isa(code[1].args[1], Float64)
end
# Make sure that the optimization doesn't apply to invalid calls
f_sqrt_wrong() = Base.Math.sqrt_llvm(1.0, 2.0)
let (ci, rt) = (code_typed(f_sqrt_wrong, Tuple{})[1])
@test rt === Union{}
end