From 96b228835ad3a9b366e572ffb4d3cebee60d0f56 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sun, 8 May 2016 21:36:44 -0700 Subject: [PATCH] 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. --- src/librustc_trans/expr.rs | 39 +------------------------------ src/librustc_trans/mir/rvalue.rs | 40 +------------------------------- 2 files changed, 2 insertions(+), 77 deletions(-) diff --git a/src/librustc_trans/expr.rs b/src/librustc_trans/expr.rs index cd11ca58689..a583c9adae6 100644 --- a/src/librustc_trans/expr.rs +++ b/src/librustc_trans/expr.rs @@ -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; @@ -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(); @@ -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, diff --git a/src/librustc_trans/mir/rvalue.rs b/src/librustc_trans/mir/rvalue.rs index 1236100a4d5..0d1c2c32ec5 100644 --- a/src/librustc_trans/mir/rvalue.rs +++ b/src/librustc_trans/mir/rvalue.rs @@ -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; @@ -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 {