Skip to content

Commit

Permalink
address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@arielb1
arielb1 committed Nov 13, 2015
1 parent e82f5d4 commit b9b45a0
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 74 deletions.
48 changes: 48 additions & 0 deletions src/librustc_mir/tcx/mod.rs
View file
Expand Up @@ -103,6 +103,31 @@ impl<'tcx> Mir<'tcx> {
}
}

pub fn binop_ty(&self,
tcx: &ty::ctxt<'tcx>,
op: BinOp,
lhs_ty: Ty<'tcx>,
rhs_ty: Ty<'tcx>)
-> Ty<'tcx>
{
// FIXME: handle SIMD correctly
match op {
BinOp::Add | BinOp::Sub | BinOp::Mul | BinOp::Div | BinOp::Rem |
BinOp::BitXor | BinOp::BitAnd | BinOp::BitOr => {
// these should be integers or floats of the same size.
assert_eq!(lhs_ty, rhs_ty);
lhs_ty
}
BinOp::Shl | BinOp::Shr => {
lhs_ty // lhs_ty can be != rhs_ty
}
BinOp::Eq | BinOp::Lt | BinOp::Le |
BinOp::Ne | BinOp::Ge | BinOp::Gt => {
tcx.types.bool
}
}
}

pub fn lvalue_ty(&self,
tcx: &ty::ctxt<'tcx>,
lvalue: &Lvalue<'tcx>)
Expand Down Expand Up @@ -138,3 +163,26 @@ impl BorrowKind {
}
}
}

impl BinOp {
pub fn to_hir_binop(self) -> hir::BinOp_ {
match self {
BinOp::Add => hir::BinOp_::BiAdd,
BinOp::Sub => hir::BinOp_::BiSub,
BinOp::Mul => hir::BinOp_::BiMul,
BinOp::Div => hir::BinOp_::BiDiv,
BinOp::Rem => hir::BinOp_::BiRem,
BinOp::BitXor => hir::BinOp_::BiBitXor,
BinOp::BitAnd => hir::BinOp_::BiBitAnd,
BinOp::BitOr => hir::BinOp_::BiBitOr,
BinOp::Shl => hir::BinOp_::BiShl,
BinOp::Shr => hir::BinOp_::BiShr,
BinOp::Eq => hir::BinOp_::BiEq,
BinOp::Ne => hir::BinOp_::BiNe,
BinOp::Lt => hir::BinOp_::BiLt,
BinOp::Gt => hir::BinOp_::BiGt,
BinOp::Le => hir::BinOp_::BiLe,
BinOp::Ge => hir::BinOp_::BiGe
}
}
}
26 changes: 14 additions & 12 deletions src/librustc_trans/trans/mir/constant.rs
View file
Expand Up @@ -16,7 +16,7 @@ use trans::common::{self, Block};
use trans::common::{C_bool, C_bytes, C_floating_f64, C_integral, C_str_slice};
use trans::type_of;

use super::operand::{OperandRef, OperandValue};
use super::operand::OperandRef;
use super::MirContext;

impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
Expand All @@ -26,19 +26,21 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
ty: Ty<'tcx>)
-> OperandRef<'tcx>
{
use super::operand::OperandValue::{Ref, Immediate};

let ccx = bcx.ccx();
let llty = type_of::type_of(ccx, ty);
let val = match *cv {
ConstVal::Float(v) => OperandValue::Imm(C_floating_f64(v, llty)),
ConstVal::Bool(v) => OperandValue::Imm(C_bool(ccx, v)),
ConstVal::Int(v) => OperandValue::Imm(C_integral(llty, v as u64, true)),
ConstVal::Uint(v) => OperandValue::Imm(C_integral(llty, v, false)),
ConstVal::Str(ref v) => OperandValue::Imm(C_str_slice(ccx, v.clone())),
ConstVal::Float(v) => Immediate(C_floating_f64(v, llty)),
ConstVal::Bool(v) => Immediate(C_bool(ccx, v)),
ConstVal::Int(v) => Immediate(C_integral(llty, v as u64, true)),
ConstVal::Uint(v) => Immediate(C_integral(llty, v, false)),
ConstVal::Str(ref v) => Immediate(C_str_slice(ccx, v.clone())),
ConstVal::ByteStr(ref v) => {
OperandValue::Imm(consts::addr_of(ccx,
C_bytes(ccx, v),
1,
"byte_str"))
Immediate(consts::addr_of(ccx,
C_bytes(ccx, v),
1,
"byte_str"))
}

ConstVal::Struct(id) | ConstVal::Tuple(id) => {
Expand All @@ -52,9 +54,9 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
Err(_) => panic!("constant eval failure"),
};
if common::type_is_immediate(bcx.ccx(), ty) {
OperandValue::Imm(llval)
Immediate(llval)
} else {
OperandValue::Ref(llval)
Ref(llval)
}
}
ConstVal::Function(_) => {
Expand Down
28 changes: 18 additions & 10 deletions src/librustc_trans/trans/mir/operand.rs
View file
Expand Up @@ -17,35 +17,43 @@ use trans::datum;

use super::{MirContext, TempRef};

/// The Rust representation of an operand's value. This is uniquely
/// determined by the operand type, but is kept as an enum as a
/// The representation of a Rust value. The enum variant is in fact
/// uniquely determined by the value's type, but is kept as a
/// safety check.
#[derive(Copy, Clone)]
pub enum OperandValue {
/// A reference to the actual operand. The data is guaranteed
/// to be valid for the operand's lifetime.
Ref(ValueRef),
/// A single LLVM value.
Imm(ValueRef),
Immediate(ValueRef),
/// A fat pointer. The first ValueRef is the data and the second
/// is the extra.
FatPtr(ValueRef, ValueRef)
}

/// An `OperandRef` is an "SSA" reference to a Rust value, along with
/// its type.
///
/// NOTE: unless you know a value's type exactly, you should not
/// generate LLVM opcodes acting on it and instead act via methods,
/// to avoid nasty edge cases. In particular, using `build::Store`
/// directly is sure to cause problems - use `store_operand` instead.
#[derive(Copy, Clone)]
pub struct OperandRef<'tcx> {
// This will be "indirect" if `appropriate_rvalue_mode` returns
// ByRef, and otherwise ByValue.
// The value.
pub val: OperandValue,

// The type of value being returned.
pub ty: Ty<'tcx>
}

impl<'tcx> OperandRef<'tcx> {
/// Asserts that this operand refers to a scalar and returns
/// a reference to its value.
pub fn immediate(self) -> ValueRef {
match self.val {
OperandValue::Imm(s) => s,
OperandValue::Immediate(s) => s,
_ => unreachable!()
}
}
Expand All @@ -56,8 +64,8 @@ impl<'tcx> OperandRef<'tcx> {
format!("OperandRef(Ref({}) @ {:?})",
bcx.val_to_string(r), self.ty)
}
OperandValue::Imm(i) => {
format!("OperandRef(Imm({}) @ {:?})",
OperandValue::Immediate(i) => {
format!("OperandRef(Immediate({}) @ {:?})",
bcx.val_to_string(i), self.ty)
}
OperandValue::FatPtr(a, d) => {
Expand Down Expand Up @@ -106,7 +114,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
ty);
let val = match datum::appropriate_rvalue_mode(bcx.ccx(), ty) {
datum::ByValue => {
OperandValue::Imm(base::load_ty(bcx, tr_lvalue.llval, ty))
OperandValue::Immediate(base::load_ty(bcx, tr_lvalue.llval, ty))
}
datum::ByRef if common::type_is_fat_ptr(bcx.tcx(), ty) => {
let (lldata, llextra) = base::load_fat_ptr(bcx, tr_lvalue.llval, ty);
Expand Down Expand Up @@ -150,7 +158,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
debug!("store_operand: operand={}", operand.repr(bcx));
match operand.val {
OperandValue::Ref(r) => base::memcpy_ty(bcx, lldest, r, operand.ty),
OperandValue::Imm(s) => base::store_ty(bcx, s, lldest, operand.ty),
OperandValue::Immediate(s) => base::store_ty(bcx, s, lldest, operand.ty),
OperandValue::FatPtr(data, extra) => {
base::store_fat_ptr(bcx, data, extra, lldest, operand.ty);
}
Expand Down
70 changes: 18 additions & 52 deletions src/librustc_trans/trans/mir/rvalue.rs
View file
Expand Up @@ -10,7 +10,6 @@

use llvm::ValueRef;
use rustc::middle::ty::{self, Ty};
use rustc_front::hir;
use rustc_mir::repr as mir;

use trans::asm;
Expand Down Expand Up @@ -47,6 +46,8 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {

mir::Rvalue::Cast(mir::CastKind::Unsize, ref operand, cast_ty) => {
if common::type_is_fat_ptr(bcx.tcx(), cast_ty) {
// into-coerce of a thin pointer to a fat pointer - just
// use the operand path.
let (bcx, temp) = self.trans_rvalue_operand(bcx, rvalue);
self.store_operand(bcx, lldest, temp);
return bcx;
Expand All @@ -59,8 +60,13 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
let operand = self.trans_operand(bcx, operand);
match operand.val {
OperandValue::FatPtr(..) => unreachable!(),
OperandValue::Imm(llval) => {
// ugly alloca.
OperandValue::Immediate(llval) => {
// unsize from an immediate structure. We don't
// really need a temporary alloca here, but
// avoiding it would require us to have
// `coerce_unsized_into` use extractvalue to
// index into the struct, and this case isn't
// important enough for it.
debug!("trans_rvalue: creating ugly alloca");
let lltemp = base::alloc_ty(bcx, operand.ty, "__unsize_temp");
base::store_ty(bcx, llval, lltemp, operand.ty);
Expand Down Expand Up @@ -165,7 +171,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
// and is a no-op at the LLVM level
operand.val
}
OperandValue::Imm(lldata) => {
OperandValue::Immediate(lldata) => {
// "standard" unsize
let (lldata, llextra) =
base::unsize_thin_ptr(bcx, lldata,
Expand Down Expand Up @@ -200,7 +206,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
// destination effectively creates a reference.
if common::type_is_sized(bcx.tcx(), ty) {
(bcx, OperandRef {
val: OperandValue::Imm(tr_lvalue.llval),
val: OperandValue::Immediate(tr_lvalue.llval),
ty: ref_ty,
})
} else {
Expand All @@ -215,7 +221,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
mir::Rvalue::Len(ref lvalue) => {
let tr_lvalue = self.trans_lvalue(bcx, lvalue);
(bcx, OperandRef {
val: OperandValue::Imm(self.lvalue_len(bcx, tr_lvalue)),
val: OperandValue::Immediate(self.lvalue_len(bcx, tr_lvalue)),
ty: bcx.tcx().types.usize,
})
}
Expand All @@ -230,7 +236,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
base::compare_fat_ptrs(bcx,
lhs_addr, lhs_extra,
rhs_addr, rhs_extra,
lhs.ty, cmp_to_hir_cmp(op),
lhs.ty, op.to_hir_binop(),
DebugLoc::None)
}
_ => unreachable!()
Expand All @@ -242,8 +248,8 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
lhs.ty, DebugLoc::None)
};
(bcx, OperandRef {
val: OperandValue::Imm(llresult),
ty: type_of_binop(bcx.tcx(), op, lhs.ty, rhs.ty),
val: OperandValue::Immediate(llresult),
ty: self.mir.binop_ty(bcx.tcx(), op, lhs.ty, rhs.ty),
})
}

Expand All @@ -261,7 +267,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
}
};
(bcx, OperandRef {
val: OperandValue::Imm(llval),
val: OperandValue::Immediate(llval),
ty: operand.ty,
})
}
Expand All @@ -281,7 +287,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
llalign,
DebugLoc::None);
(bcx, OperandRef {
val: OperandValue::Imm(llval),
val: OperandValue::Immediate(llval),
ty: box_ty,
})
}
Expand Down Expand Up @@ -388,7 +394,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
mir::BinOp::Eq | mir::BinOp::Lt | mir::BinOp::Gt |
mir::BinOp::Ne | mir::BinOp::Le | mir::BinOp::Ge => {
base::compare_scalar_types(bcx, lhs, rhs, input_ty,
cmp_to_hir_cmp(op), debug_loc)
op.to_hir_binop(), debug_loc)
}
}
}
Expand All @@ -413,43 +419,3 @@ pub fn rvalue_creates_operand<'tcx>(rvalue: &mir::Rvalue<'tcx>) -> bool {

// (*) this is only true if the type is suitable
}

fn cmp_to_hir_cmp(op: mir::BinOp) -> hir::BinOp_ {
match op {
mir::BinOp::Eq => hir::BiEq,
mir::BinOp::Ne => hir::BiNe,
mir::BinOp::Lt => hir::BiLt,
mir::BinOp::Le => hir::BiLe,
mir::BinOp::Gt => hir::BiGt,
mir::BinOp::Ge => hir::BiGe,
_ => unreachable!()
}
}

/// FIXME(nikomatsakis): I don't think this function should go here
fn type_of_binop<'tcx>(
tcx: &ty::ctxt<'tcx>,
op: mir::BinOp,
lhs_ty: Ty<'tcx>,
rhs_ty: Ty<'tcx>)
-> Ty<'tcx>
{
match op {
mir::BinOp::Add | mir::BinOp::Sub |
mir::BinOp::Mul | mir::BinOp::Div | mir::BinOp::Rem |
mir::BinOp::BitXor | mir::BinOp::BitAnd | mir::BinOp::BitOr => {
// these should be integers or floats of the same size. We
// probably want to dump all ops in some intrinsics framework
// someday.
assert_eq!(lhs_ty, rhs_ty);
lhs_ty
}
mir::BinOp::Shl | mir::BinOp::Shr => {
lhs_ty // lhs_ty can be != rhs_ty
}
mir::BinOp::Eq | mir::BinOp::Lt | mir::BinOp::Le |
mir::BinOp::Ne | mir::BinOp::Ge | mir::BinOp::Gt => {
tcx.types.bool
}
}
}

0 comments on commit b9b45a0

Please sign in to comment.