Skip to content

Commit b9b45a0

Browse files
arielb1arielb1
committed
address review comments
1 parent e82f5d4 commit b9b45a0

File tree

4 files changed

+98
-74
lines changed

4 files changed

+98
-74
lines changed

‎src/librustc_mir/tcx/mod.rs‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,31 @@ impl<'tcx> Mir<'tcx> {
103103
}
104104
}
105105

106+
pub fn binop_ty(&self,
107+
tcx: &ty::ctxt<'tcx>,
108+
op: BinOp,
109+
lhs_ty: Ty<'tcx>,
110+
rhs_ty: Ty<'tcx>)
111+
-> Ty<'tcx>
112+
{
113+
// FIXME: handle SIMD correctly
114+
match op {
115+
BinOp::Add | BinOp::Sub | BinOp::Mul | BinOp::Div | BinOp::Rem |
116+
BinOp::BitXor | BinOp::BitAnd | BinOp::BitOr => {
117+
// these should be integers or floats of the same size.
118+
assert_eq!(lhs_ty, rhs_ty);
119+
lhs_ty
120+
}
121+
BinOp::Shl | BinOp::Shr => {
122+
lhs_ty // lhs_ty can be != rhs_ty
123+
}
124+
BinOp::Eq | BinOp::Lt | BinOp::Le |
125+
BinOp::Ne | BinOp::Ge | BinOp::Gt => {
126+
tcx.types.bool
127+
}
128+
}
129+
}
130+
106131
pub fn lvalue_ty(&self,
107132
tcx: &ty::ctxt<'tcx>,
108133
lvalue: &Lvalue<'tcx>)
@@ -138,3 +163,26 @@ impl BorrowKind {
138163
}
139164
}
140165
}
166+
167+
impl BinOp {
168+
pub fn to_hir_binop(self) -> hir::BinOp_ {
169+
match self {
170+
BinOp::Add => hir::BinOp_::BiAdd,
171+
BinOp::Sub => hir::BinOp_::BiSub,
172+
BinOp::Mul => hir::BinOp_::BiMul,
173+
BinOp::Div => hir::BinOp_::BiDiv,
174+
BinOp::Rem => hir::BinOp_::BiRem,
175+
BinOp::BitXor => hir::BinOp_::BiBitXor,
176+
BinOp::BitAnd => hir::BinOp_::BiBitAnd,
177+
BinOp::BitOr => hir::BinOp_::BiBitOr,
178+
BinOp::Shl => hir::BinOp_::BiShl,
179+
BinOp::Shr => hir::BinOp_::BiShr,
180+
BinOp::Eq => hir::BinOp_::BiEq,
181+
BinOp::Ne => hir::BinOp_::BiNe,
182+
BinOp::Lt => hir::BinOp_::BiLt,
183+
BinOp::Gt => hir::BinOp_::BiGt,
184+
BinOp::Le => hir::BinOp_::BiLe,
185+
BinOp::Ge => hir::BinOp_::BiGe
186+
}
187+
}
188+
}

‎src/librustc_trans/trans/mir/constant.rs‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use trans::common::{self, Block};
1616
use trans::common::{C_bool, C_bytes, C_floating_f64, C_integral, C_str_slice};
1717
use trans::type_of;
1818

19-
use super::operand::{OperandRef, OperandValue};
19+
use super::operand::OperandRef;
2020
use super::MirContext;
2121

2222
impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
@@ -26,19 +26,21 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
2626
ty: Ty<'tcx>)
2727
-> OperandRef<'tcx>
2828
{
29+
use super::operand::OperandValue::{Ref, Immediate};
30+
2931
let ccx = bcx.ccx();
3032
let llty = type_of::type_of(ccx, ty);
3133
let val = match *cv {
32-
ConstVal::Float(v) => OperandValue::Imm(C_floating_f64(v, llty)),
33-
ConstVal::Bool(v) => OperandValue::Imm(C_bool(ccx, v)),
34-
ConstVal::Int(v) => OperandValue::Imm(C_integral(llty, v as u64, true)),
35-
ConstVal::Uint(v) => OperandValue::Imm(C_integral(llty, v, false)),
36-
ConstVal::Str(ref v) => OperandValue::Imm(C_str_slice(ccx, v.clone())),
34+
ConstVal::Float(v) => Immediate(C_floating_f64(v, llty)),
35+
ConstVal::Bool(v) => Immediate(C_bool(ccx, v)),
36+
ConstVal::Int(v) => Immediate(C_integral(llty, v as u64, true)),
37+
ConstVal::Uint(v) => Immediate(C_integral(llty, v, false)),
38+
ConstVal::Str(ref v) => Immediate(C_str_slice(ccx, v.clone())),
3739
ConstVal::ByteStr(ref v) => {
38-
OperandValue::Imm(consts::addr_of(ccx,
39-
C_bytes(ccx, v),
40-
1,
41-
"byte_str"))
40+
Immediate(consts::addr_of(ccx,
41+
C_bytes(ccx, v),
42+
1,
43+
"byte_str"))
4244
}
4345

4446
ConstVal::Struct(id) | ConstVal::Tuple(id) => {
@@ -52,9 +54,9 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
5254
Err(_) => panic!("constant eval failure"),
5355
};
5456
if common::type_is_immediate(bcx.ccx(), ty) {
55-
OperandValue::Imm(llval)
57+
Immediate(llval)
5658
} else {
57-
OperandValue::Ref(llval)
59+
Ref(llval)
5860
}
5961
}
6062
ConstVal::Function(_) => {

‎src/librustc_trans/trans/mir/operand.rs‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,35 +17,43 @@ use trans::datum;
1717

1818
use super::{MirContext, TempRef};
1919

20-
/// The Rust representation of an operand's value. This is uniquely
21-
/// determined by the operand type, but is kept as an enum as a
20+
/// The representation of a Rust value. The enum variant is in fact
21+
/// uniquely determined by the value's type, but is kept as a
2222
/// safety check.
2323
#[derive(Copy, Clone)]
2424
pub enum OperandValue {
2525
/// A reference to the actual operand. The data is guaranteed
2626
/// to be valid for the operand's lifetime.
2727
Ref(ValueRef),
2828
/// A single LLVM value.
29-
Imm(ValueRef),
29+
Immediate(ValueRef),
3030
/// A fat pointer. The first ValueRef is the data and the second
3131
/// is the extra.
3232
FatPtr(ValueRef, ValueRef)
3333
}
3434

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

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

4551
impl<'tcx> OperandRef<'tcx> {
52+
/// Asserts that this operand refers to a scalar and returns
53+
/// a reference to its value.
4654
pub fn immediate(self) -> ValueRef {
4755
match self.val {
48-
OperandValue::Imm(s) => s,
56+
OperandValue::Immediate(s) => s,
4957
_ => unreachable!()
5058
}
5159
}
@@ -56,8 +64,8 @@ impl<'tcx> OperandRef<'tcx> {
5664
format!("OperandRef(Ref({}) @ {:?})",
5765
bcx.val_to_string(r), self.ty)
5866
}
59-
OperandValue::Imm(i) => {
60-
format!("OperandRef(Imm({}) @ {:?})",
67+
OperandValue::Immediate(i) => {
68+
format!("OperandRef(Immediate({}) @ {:?})",
6169
bcx.val_to_string(i), self.ty)
6270
}
6371
OperandValue::FatPtr(a, d) => {
@@ -106,7 +114,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
106114
ty);
107115
let val = match datum::appropriate_rvalue_mode(bcx.ccx(), ty) {
108116
datum::ByValue => {
109-
OperandValue::Imm(base::load_ty(bcx, tr_lvalue.llval, ty))
117+
OperandValue::Immediate(base::load_ty(bcx, tr_lvalue.llval, ty))
110118
}
111119
datum::ByRef if common::type_is_fat_ptr(bcx.tcx(), ty) => {
112120
let (lldata, llextra) = base::load_fat_ptr(bcx, tr_lvalue.llval, ty);
@@ -150,7 +158,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
150158
debug!("store_operand: operand={}", operand.repr(bcx));
151159
match operand.val {
152160
OperandValue::Ref(r) => base::memcpy_ty(bcx, lldest, r, operand.ty),
153-
OperandValue::Imm(s) => base::store_ty(bcx, s, lldest, operand.ty),
161+
OperandValue::Immediate(s) => base::store_ty(bcx, s, lldest, operand.ty),
154162
OperandValue::FatPtr(data, extra) => {
155163
base::store_fat_ptr(bcx, data, extra, lldest, operand.ty);
156164
}

‎src/librustc_trans/trans/mir/rvalue.rs‎

Lines changed: 18 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
use llvm::ValueRef;
1212
use rustc::middle::ty::{self, Ty};
13-
use rustc_front::hir;
1413
use rustc_mir::repr as mir;
1514

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

4847
mir::Rvalue::Cast(mir::CastKind::Unsize, ref operand, cast_ty) => {
4948
if common::type_is_fat_ptr(bcx.tcx(), cast_ty) {
49+
// into-coerce of a thin pointer to a fat pointer - just
50+
// use the operand path.
5051
let (bcx, temp) = self.trans_rvalue_operand(bcx, rvalue);
5152
self.store_operand(bcx, lldest, temp);
5253
return bcx;
@@ -59,8 +60,13 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
5960
let operand = self.trans_operand(bcx, operand);
6061
match operand.val {
6162
OperandValue::FatPtr(..) => unreachable!(),
62-
OperandValue::Imm(llval) => {
63-
// ugly alloca.
63+
OperandValue::Immediate(llval) => {
64+
// unsize from an immediate structure. We don't
65+
// really need a temporary alloca here, but
66+
// avoiding it would require us to have
67+
// `coerce_unsized_into` use extractvalue to
68+
// index into the struct, and this case isn't
69+
// important enough for it.
6470
debug!("trans_rvalue: creating ugly alloca");
6571
let lltemp = base::alloc_ty(bcx, operand.ty, "__unsize_temp");
6672
base::store_ty(bcx, llval, lltemp, operand.ty);
@@ -165,7 +171,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
165171
// and is a no-op at the LLVM level
166172
operand.val
167173
}
168-
OperandValue::Imm(lldata) => {
174+
OperandValue::Immediate(lldata) => {
169175
// "standard" unsize
170176
let (lldata, llextra) =
171177
base::unsize_thin_ptr(bcx, lldata,
@@ -200,7 +206,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
200206
// destination effectively creates a reference.
201207
if common::type_is_sized(bcx.tcx(), ty) {
202208
(bcx, OperandRef {
203-
val: OperandValue::Imm(tr_lvalue.llval),
209+
val: OperandValue::Immediate(tr_lvalue.llval),
204210
ty: ref_ty,
205211
})
206212
} else {
@@ -215,7 +221,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
215221
mir::Rvalue::Len(ref lvalue) => {
216222
let tr_lvalue = self.trans_lvalue(bcx, lvalue);
217223
(bcx, OperandRef {
218-
val: OperandValue::Imm(self.lvalue_len(bcx, tr_lvalue)),
224+
val: OperandValue::Immediate(self.lvalue_len(bcx, tr_lvalue)),
219225
ty: bcx.tcx().types.usize,
220226
})
221227
}
@@ -230,7 +236,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
230236
base::compare_fat_ptrs(bcx,
231237
lhs_addr, lhs_extra,
232238
rhs_addr, rhs_extra,
233-
lhs.ty, cmp_to_hir_cmp(op),
239+
lhs.ty, op.to_hir_binop(),
234240
DebugLoc::None)
235241
}
236242
_ => unreachable!()
@@ -242,8 +248,8 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
242248
lhs.ty, DebugLoc::None)
243249
};
244250
(bcx, OperandRef {
245-
val: OperandValue::Imm(llresult),
246-
ty: type_of_binop(bcx.tcx(), op, lhs.ty, rhs.ty),
251+
val: OperandValue::Immediate(llresult),
252+
ty: self.mir.binop_ty(bcx.tcx(), op, lhs.ty, rhs.ty),
247253
})
248254
}
249255

@@ -261,7 +267,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
261267
}
262268
};
263269
(bcx, OperandRef {
264-
val: OperandValue::Imm(llval),
270+
val: OperandValue::Immediate(llval),
265271
ty: operand.ty,
266272
})
267273
}
@@ -281,7 +287,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
281287
llalign,
282288
DebugLoc::None);
283289
(bcx, OperandRef {
284-
val: OperandValue::Imm(llval),
290+
val: OperandValue::Immediate(llval),
285291
ty: box_ty,
286292
})
287293
}
@@ -388,7 +394,7 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
388394
mir::BinOp::Eq | mir::BinOp::Lt | mir::BinOp::Gt |
389395
mir::BinOp::Ne | mir::BinOp::Le | mir::BinOp::Ge => {
390396
base::compare_scalar_types(bcx, lhs, rhs, input_ty,
391-
cmp_to_hir_cmp(op), debug_loc)
397+
op.to_hir_binop(), debug_loc)
392398
}
393399
}
394400
}
@@ -413,43 +419,3 @@ pub fn rvalue_creates_operand<'tcx>(rvalue: &mir::Rvalue<'tcx>) -> bool {
413419

414420
// (*) this is only true if the type is suitable
415421
}
416-
417-
fn cmp_to_hir_cmp(op: mir::BinOp) -> hir::BinOp_ {
418-
match op {
419-
mir::BinOp::Eq => hir::BiEq,
420-
mir::BinOp::Ne => hir::BiNe,
421-
mir::BinOp::Lt => hir::BiLt,
422-
mir::BinOp::Le => hir::BiLe,
423-
mir::BinOp::Gt => hir::BiGt,
424-
mir::BinOp::Ge => hir::BiGe,
425-
_ => unreachable!()
426-
}
427-
}
428-
429-
/// FIXME(nikomatsakis): I don't think this function should go here
430-
fn type_of_binop<'tcx>(
431-
tcx: &ty::ctxt<'tcx>,
432-
op: mir::BinOp,
433-
lhs_ty: Ty<'tcx>,
434-
rhs_ty: Ty<'tcx>)
435-
-> Ty<'tcx>
436-
{
437-
match op {
438-
mir::BinOp::Add | mir::BinOp::Sub |
439-
mir::BinOp::Mul | mir::BinOp::Div | mir::BinOp::Rem |
440-
mir::BinOp::BitXor | mir::BinOp::BitAnd | mir::BinOp::BitOr => {
441-
// these should be integers or floats of the same size. We
442-
// probably want to dump all ops in some intrinsics framework
443-
// someday.
444-
assert_eq!(lhs_ty, rhs_ty);
445-
lhs_ty
446-
}
447-
mir::BinOp::Shl | mir::BinOp::Shr => {
448-
lhs_ty // lhs_ty can be != rhs_ty
449-
}
450-
mir::BinOp::Eq | mir::BinOp::Lt | mir::BinOp::Le |
451-
mir::BinOp::Ne | mir::BinOp::Ge | mir::BinOp::Gt => {
452-
tcx.types.bool
453-
}
454-
}
455-
}

0 commit comments

Comments
 (0)