Skip to content

Commit

Permalink
trans: Always lower to frem
Browse files Browse the repository at this point in the history 
Long ago LLVM unfortunately didn't handle the 32-bit MSVC case of `frem` where
it can't be lowered to `fmodf` because that symbol doesn't exist. That was since
fixed in http://reviews.llvm.org/D12099 (landed as r246615) and was released in
what appears to be LLVM 3.8. Now that we're using that branch of LLVM let's
remove our own hacks and help LLVM optimize a little better by giving it
knowledge about what we're doing.
  • Loading branch information
@alexcrichton
alexcrichton committed May 9, 2016
1 parent cae42a4 commit 96b2288
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 77 deletions.
39 changes: 1 addition & 38 deletions src/librustc_trans/expr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ use cleanup::{self, CleanupMethods, DropHintMethods};
use common::*;
use datum::*;
use debuginfo::{self, DebugLoc, ToDebugLoc};
use declare;
use glue;
use machine;
use tvec;
Expand Down Expand Up @@ -1591,7 +1590,6 @@ fn trans_scalar_binop<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
{
let _icx = push_ctxt("trans_scalar_binop");

let tcx = bcx.tcx();
let lhs_t = lhs.ty;
assert!(!lhs_t.is_simd());
let is_float = lhs_t.is_fp();
Expand Down Expand Up @@ -1654,42 +1652,7 @@ fn trans_scalar_binop<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
}
hir::BiRem => {
if is_float {
// LLVM currently always lowers the `frem` instructions appropriate
// library calls typically found in libm. Notably f64 gets wired up
// to `fmod` and f32 gets wired up to `fmodf`. Inconveniently for
// us, 32-bit MSVC does not actually have a `fmodf` symbol, it's
// instead just an inline function in a header that goes up to a
// f64, uses `fmod`, and then comes back down to a f32.
//
// Although LLVM knows that `fmodf` doesn't exist on MSVC, it will
// still unconditionally lower frem instructions over 32-bit floats
// to a call to `fmodf`. To work around this we special case MSVC
// 32-bit float rem instructions and instead do the call out to
// `fmod` ourselves.
//
// Note that this is currently duplicated with src/libcore/ops.rs
// which does the same thing, and it would be nice to perhaps unify
// these two implementations on day! Also note that we call `fmod`
// for both 32 and 64-bit floats because if we emit any FRem
// instruction at all then LLVM is capable of optimizing it into a
// 32-bit FRem (which we're trying to avoid).
let use_fmod = tcx.sess.target.target.options.is_like_msvc &&
tcx.sess.target.target.arch == "x86";
if use_fmod {
let f64t = Type::f64(bcx.ccx());
let fty = Type::func(&[f64t, f64t], &f64t);
let llfn = declare::declare_cfn(bcx.ccx(), "fmod", fty);
if lhs_t == tcx.types.f32 {
let lhs = FPExt(bcx, lhs, f64t);
let rhs = FPExt(bcx, rhs, f64t);
let res = Call(bcx, llfn, &[lhs, rhs], binop_debug_loc);
FPTrunc(bcx, res, Type::f32(bcx.ccx()))
} else {
Call(bcx, llfn, &[lhs, rhs], binop_debug_loc)
}
} else {
FRem(bcx, lhs, rhs, binop_debug_loc)
}
FRem(bcx, lhs, rhs, binop_debug_loc)
} else {
// Only zero-check integers; fp %0 is NaN
bcx = base::fail_if_zero_or_overflows(bcx,
Expand Down
40 changes: 1 addition & 39 deletions src/librustc_trans/mir/rvalue.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,8 @@ use callee::Callee;
use common::{self, C_uint, BlockAndBuilder, Result};
use datum::{Datum, Lvalue};
use debuginfo::DebugLoc;
use declare;
use adt;
use machine;
use type_::Type;
use type_of;
use tvec;
use value::Value;
Expand Down Expand Up @@ -529,43 +527,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
bcx.udiv(lhs, rhs)
},
mir::BinOp::Rem => if is_float {
// LLVM currently always lowers the `frem` instructions appropriate
// library calls typically found in libm. Notably f64 gets wired up
// to `fmod` and f32 gets wired up to `fmodf`. Inconveniently for
// us, 32-bit MSVC does not actually have a `fmodf` symbol, it's
// instead just an inline function in a header that goes up to a
// f64, uses `fmod`, and then comes back down to a f32.
//
// Although LLVM knows that `fmodf` doesn't exist on MSVC, it will
// still unconditionally lower frem instructions over 32-bit floats
// to a call to `fmodf`. To work around this we special case MSVC
// 32-bit float rem instructions and instead do the call out to
// `fmod` ourselves.
//
// Note that this is currently duplicated with src/libcore/ops.rs
// which does the same thing, and it would be nice to perhaps unify
// these two implementations one day! Also note that we call `fmod`
// for both 32 and 64-bit floats because if we emit any FRem
// instruction at all then LLVM is capable of optimizing it into a
// 32-bit FRem (which we're trying to avoid).
let tcx = bcx.tcx();
let use_fmod = tcx.sess.target.target.options.is_like_msvc &&
tcx.sess.target.target.arch == "x86";
if use_fmod {
let f64t = Type::f64(bcx.ccx());
let fty = Type::func(&[f64t, f64t], &f64t);
let llfn = declare::declare_cfn(bcx.ccx(), "fmod", fty);
if input_ty == tcx.types.f32 {
let lllhs = bcx.fpext(lhs, f64t);
let llrhs = bcx.fpext(rhs, f64t);
let llres = bcx.call(llfn, &[lllhs, llrhs], None);
bcx.fptrunc(llres, Type::f32(bcx.ccx()))
} else {
bcx.call(llfn, &[lhs, rhs], None)
}
} else {
bcx.frem(lhs, rhs)
}
bcx.frem(lhs, rhs)
} else if is_signed {
bcx.srem(lhs, rhs)
} else {
Expand Down

0 comments on commit 96b2288

Please sign in to comment.