Skip to content

Commit

Permalink
use LLVM intrinsic for sqrt. helps #6112
Browse files Browse the repository at this point in the history
  • Loading branch information
JeffBezanson committed Mar 13, 2014
1 parent 9785017 commit 244ec92
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 4 deletions.
2 changes: 1 addition & 1 deletion base/boot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ export
#checked_smul, checked_ssub, checked_uadd, checked_umul, checked_usub,
#nan_dom_err, copysign_float, ctlz_int, ctpop_int, cttz_int,
#div_float, eq_float, eq_int, eqfsi64, eqfui64, flipsign_int, select_value,
#fpext64, fpiseq, fpislt, fpsiround, fpuiround, fptosi, fptoui,
#sqrt_llvm, fpext64, fpiseq, fpislt, fpsiround, fpuiround, fptosi, fptoui,
#fptrunc32, le_float, lefsi64, lefui64, lesif64,
#leuif64, lshr_int, lt_float, ltfsi64, ltfui64, ltsif64, ltuif64, mul_float,
#mul_int, ne_float, ne_int, neg_float, neg_int, not_int, or_int, rem_float,
Expand Down
9 changes: 7 additions & 2 deletions base/math.jl
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import Base: log, exp, sin, cos, tan, sinh, cosh, tanh, asin,
acos, atan, asinh, acosh, atanh, sqrt, log2, log10,
max, min, minmax, ceil, floor, trunc, round, ^, exp2, exp10

import Core.Intrinsics.nan_dom_err
import Core.Intrinsics: nan_dom_err, sqrt_llvm, box, unbox

# non-type specific math functions

Expand Down Expand Up @@ -272,7 +272,7 @@ exp10(x::Integer) = exp10(float(x))

# functions that return NaN on non-NaN argument for domain error
for f in (:sin, :cos, :tan, :asin, :acos, :acosh, :atanh, :log, :log2, :log10,
:lgamma, :sqrt, :log1p)
:lgamma, :log1p)
@eval begin
($f)(x::Float64) = nan_dom_err(ccall(($(string(f)),libm), Float64, (Float64,), x), x)
($f)(x::Float32) = nan_dom_err(ccall(($(string(f,"f")),libm), Float32, (Float32,), x), x)
Expand All @@ -281,6 +281,11 @@ for f in (:sin, :cos, :tan, :asin, :acos, :acosh, :atanh, :log, :log2, :log10,
end
end

sqrt(x::Float64) = nan_dom_err(box(Float64,sqrt_llvm(unbox(Float64,x))), x)
sqrt(x::Float32) = nan_dom_err(box(Float32,sqrt_llvm(unbox(Float32,x))), x)
sqrt(x::Real) = sqrt(float(x))
@vectorize_1arg Number sqrt

for f in (:ceil, :trunc, :significand) # :rint, :nearbyint
@eval begin
($f)(x::Float64) = ccall(($(string(f)),libm), Float64, (Float64,), x)
Expand Down
9 changes: 8 additions & 1 deletion src/intrinsics.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ namespace JL_I {
nan_dom_err,
// functions
abs_float, copysign_float, flipsign_int, select_value,
sqrt_llvm,
// pointer access
pointerref, pointerset, pointertoref,
// c interface
Expand Down Expand Up @@ -1243,6 +1244,12 @@ static Value *emit_intrinsic(intrinsic f, jl_value_t **args, size_t nargs,
HANDLE(jl_alloca,1) {
return builder.CreateAlloca(IntegerType::get(jl_LLVMContext, 8),JL_INT(x));
}
HANDLE(sqrt_llvm,1) {
x = FP(x);
return builder.CreateCall(Intrinsic::getDeclaration(jl_Module, Intrinsic::sqrt,
ArrayRef<Type*>(x->getType())),
x);
}
default:
assert(false);
}
Expand Down Expand Up @@ -1321,7 +1328,7 @@ extern "C" void jl_init_intrinsic_functions(void)
ADD_I(uitofp); ADD_I(sitofp);
ADD_I(fptrunc); ADD_I(fpext);
ADD_I(abs_float); ADD_I(copysign_float);
ADD_I(flipsign_int); ADD_I(select_value);
ADD_I(flipsign_int); ADD_I(select_value); ADD_I(sqrt_llvm);
ADD_I(pointerref); ADD_I(pointerset); ADD_I(pointertoref);
ADD_I(checked_sadd); ADD_I(checked_uadd);
ADD_I(checked_ssub); ADD_I(checked_usub);
Expand Down

3 comments on commit 244ec92

@simonster
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem safe? The LLVM language reference says that the intrinsic has undefined behavior for negative numbers, and indeed, with this change:

julia> f() = sqrt(-1.0);

julia> f()
0.0

@JeffBezanson
Copy link
Member Author

Choose a reason for hiding this comment

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

Oh right, I knew there was some catch I forgot about. Can be fixed.

@quinnj
Copy link
Member

@quinnj quinnj commented on 244ec92 Oct 6, 2014

Choose a reason for hiding this comment

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

Not sure if this is important/relevant at all, but I came across this: http://reviews.llvm.org/rL218803

Please sign in to comment.