Skip to content

Commit

Permalink
Rollup merge of rust-lang#33508 - alexcrichton:always-lower-frem, r=n…
Browse files Browse the repository at this point in the history 
…ikomatsakis

trans: Always lower to `frem`

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
@eddyb
eddyb committed May 13, 2016
2 parents be92e9b + 96b2288 commit 6859599
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 @@ -1593,7 +1592,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 @@ -1656,42 +1654,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 @@ -531,43 +529,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 6859599

Please sign in to comment.