Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Promoteds are statics and statics have a place, not just a value #52597

Merged
merged 7 commits into from
Jul 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 3 additions & 16 deletions src/librustc/ich/impls_mir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,9 @@ impl<'a, 'gcx> HashStable<StableHashingContext<'a>> for mir::Place<'gcx> {
mir::Place::Static(ref statik) => {
statik.hash_stable(hcx, hasher);
}
mir::Place::Promoted(ref promoted) => {
promoted.hash_stable(hcx, hasher);
}
mir::Place::Projection(ref place_projection) => {
place_projection.hash_stable(hcx, hasher);
}
Expand Down Expand Up @@ -527,22 +530,6 @@ impl_stable_hash_for!(enum mir::NullOp {

impl_stable_hash_for!(struct mir::Constant<'tcx> { span, ty, literal });

impl<'a, 'gcx> HashStable<StableHashingContext<'a>> for mir::Literal<'gcx> {
fn hash_stable<W: StableHasherResult>(&self,
hcx: &mut StableHashingContext<'a>,
hasher: &mut StableHasher<W>) {
mem::discriminant(self).hash_stable(hcx, hasher);
match *self {
mir::Literal::Value { ref value } => {
value.hash_stable(hcx, hasher);
}
mir::Literal::Promoted { index } => {
index.hash_stable(hcx, hasher);
}
}
}
}

impl_stable_hash_for!(struct mir::Location { block, statement_index });

impl_stable_hash_for!(struct mir::BorrowCheckResult<'tcx> {
Expand Down
54 changes: 8 additions & 46 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1701,6 +1701,9 @@ pub enum Place<'tcx> {
/// static or static mut variable
Static(Box<Static<'tcx>>),

/// Constant code promoted to an injected static
Promoted(Box<(Promoted, Ty<'tcx>)>),

/// projection out of a place (access a field, deref a pointer, etc)
Projection(Box<PlaceProjection<'tcx>>),
}
Expand Down Expand Up @@ -1810,6 +1813,7 @@ impl<'tcx> Debug for Place<'tcx> {
ty::tls::with(|tcx| tcx.item_path_str(def_id)),
ty
),
Promoted(ref promoted) => write!(fmt, "({:?}: {:?})", promoted.0, promoted.1),
Projection(ref data) => match data.elem {
ProjectionElem::Downcast(ref adt_def, index) => {
write!(fmt, "({:?} as {})", data.base, adt_def.variants[index].name)
Expand Down Expand Up @@ -1910,9 +1914,7 @@ impl<'tcx> Operand<'tcx> {
Operand::Constant(box Constant {
span,
ty,
literal: Literal::Value {
value: ty::Const::zero_sized(tcx, ty),
},
literal: ty::Const::zero_sized(tcx, ty),
})
}

Expand Down Expand Up @@ -2200,38 +2202,15 @@ impl<'tcx> Debug for Rvalue<'tcx> {
pub struct Constant<'tcx> {
pub span: Span,
pub ty: Ty<'tcx>,
pub literal: Literal<'tcx>,
pub literal: &'tcx ty::Const<'tcx>,
}

newtype_index!(Promoted { DEBUG_FORMAT = "promoted[{}]" });

#[derive(Clone, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)]
pub enum Literal<'tcx> {
Value {
value: &'tcx ty::Const<'tcx>,
},
Promoted {
// Index into the `promoted` vector of `Mir`.
index: Promoted,
},
}

impl<'tcx> Debug for Constant<'tcx> {
fn fmt(&self, fmt: &mut Formatter) -> fmt::Result {
write!(fmt, "{:?}", self.literal)
}
}

impl<'tcx> Debug for Literal<'tcx> {
fn fmt(&self, fmt: &mut Formatter) -> fmt::Result {
use self::Literal::*;
match *self {
Value { value } => {
write!(fmt, "const ")?;
fmt_const_val(fmt, value)
}
Promoted { index } => write!(fmt, "{:?}", index),
}
write!(fmt, "const ")?;
fmt_const_val(fmt, self.literal)
}
}

Expand Down Expand Up @@ -2918,20 +2897,3 @@ impl<'tcx> TypeFoldable<'tcx> for Constant<'tcx> {
self.ty.visit_with(visitor) || self.literal.visit_with(visitor)
}
}

impl<'tcx> TypeFoldable<'tcx> for Literal<'tcx> {
fn super_fold_with<'gcx: 'tcx, F: TypeFolder<'gcx, 'tcx>>(&self, folder: &mut F) -> Self {
match *self {
Literal::Value { value } => Literal::Value {
value: value.fold_with(folder),
},
Literal::Promoted { index } => Literal::Promoted { index },
}
}
fn super_visit_with<V: TypeVisitor<'tcx>>(&self, visitor: &mut V) -> bool {
match *self {
Literal::Value { value } => value.visit_with(visitor),
Literal::Promoted { .. } => false,
}
}
}
1 change: 1 addition & 0 deletions src/librustc/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ impl<'tcx> Place<'tcx> {
match *self {
Place::Local(index) =>
PlaceTy::Ty { ty: local_decls.local_decls()[index].ty },
Place::Promoted(ref data) => PlaceTy::Ty { ty: data.1 },
Place::Static(ref data) =>
PlaceTy::Ty { ty: data.ty },
Place::Projection(ref proj) =>
Expand Down
22 changes: 4 additions & 18 deletions src/librustc/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,12 +191,6 @@ macro_rules! make_mir_visitor {
self.super_constant(constant, location);
}

fn visit_literal(&mut self,
literal: & $($mutability)* Literal<'tcx>,
location: Location) {
self.super_literal(literal, location);
}

fn visit_def_id(&mut self,
def_id: & $($mutability)* DefId,
_: Location) {
Expand Down Expand Up @@ -648,6 +642,9 @@ macro_rules! make_mir_visitor {
Place::Static(ref $($mutability)* static_) => {
self.visit_static(static_, context, location);
}
Place::Promoted(ref $($mutability)* promoted) => {
self.visit_ty(& $($mutability)* promoted.1, TyContext::Location(location));
},
Place::Projection(ref $($mutability)* proj) => {
self.visit_projection(proj, context, location);
}
Expand Down Expand Up @@ -748,18 +745,7 @@ macro_rules! make_mir_visitor {

self.visit_span(span);
self.visit_ty(ty, TyContext::Location(location));
self.visit_literal(literal, location);
}

fn super_literal(&mut self,
literal: & $($mutability)* Literal<'tcx>,
location: Location) {
match *literal {
Literal::Value { ref $($mutability)* value } => {
self.visit_const(value, location);
}
Literal::Promoted { index: _ } => {}
}
self.visit_const(literal, location);
}

fn super_def_id(&mut self, _def_id: & $($mutability)* DefId) {
Expand Down
8 changes: 5 additions & 3 deletions src/librustc_codegen_llvm/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,9 +242,11 @@ pub fn get_static(cx: &CodegenCx, def_id: DefId) -> ValueRef {
g
}

pub fn codegen_static<'a, 'tcx>(cx: &CodegenCx<'a, 'tcx>,
def_id: DefId,
is_mutable: bool) {
pub fn codegen_static<'a, 'tcx>(
cx: &CodegenCx<'a, 'tcx>,
def_id: DefId,
is_mutable: bool,
) {
unsafe {
let attrs = cx.tcx.codegen_fn_attrs(def_id);

Expand Down
28 changes: 27 additions & 1 deletion src/librustc_codegen_llvm/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -507,14 +507,40 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> {
// promotes any complex rvalues to constants.
if i == 2 && intrinsic.unwrap().starts_with("simd_shuffle") {
match *arg {
// The shuffle array argument is usually not an explicit constant,
// but specified directly in the code. This means it gets promoted
// and we can then extract the value by evaluating the promoted.
mir::Operand::Copy(mir::Place::Promoted(box(index, ty))) |
mir::Operand::Move(mir::Place::Promoted(box(index, ty))) => {
let param_env = ty::ParamEnv::reveal_all();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be something like self.param_env (minor future hazard if we start deduplicating instances).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR already landed.Does anyone take care of fixing this?^^

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is rustc_codegen_llvm... we don't have a param env, everything is monomorphized

let cid = mir::interpret::GlobalId {
instance: self.instance,
promoted: Some(index),
};
let c = bx.tcx().const_eval(param_env.and(cid));
let (llval, ty) = self.simd_shuffle_indices(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the point of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SIMD shuffle indices are an array, which usually is specified inline, and not via an explicit constant. so the array is promoted and we need to read it here

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. A comment to that effect would be great.

&bx,
terminator.source_info.span,
ty,
c,
);
return OperandRef {
val: Immediate(llval),
layout: bx.cx.layout_of(ty),
};

},
mir::Operand::Copy(_) |
mir::Operand::Move(_) => {
span_bug!(span, "shuffle indices must be constant");
}
mir::Operand::Constant(ref constant) => {
let c = self.eval_mir_constant(&bx, constant);
let (llval, ty) = self.simd_shuffle_indices(
&bx,
constant,
constant.span,
constant.ty,
c,
);
return OperandRef {
val: Immediate(llval),
Expand Down
33 changes: 12 additions & 21 deletions src/librustc_codegen_llvm/mir/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use consts;
use type_of::LayoutLlvmExt;
use type_::Type;
use syntax::ast::Mutability;
use syntax::codemap::Span;

use super::super::callee;
use super::FunctionCx;
Expand Down Expand Up @@ -117,13 +118,12 @@ pub fn const_alloc_to_llvm(cx: &CodegenCx, alloc: &Allocation) -> ValueRef {

pub fn codegen_static_initializer<'a, 'tcx>(
cx: &CodegenCx<'a, 'tcx>,
def_id: DefId)
-> Result<(ValueRef, &'tcx Allocation), Lrc<ConstEvalErr<'tcx>>>
{
def_id: DefId,
) -> Result<(ValueRef, &'tcx Allocation), Lrc<ConstEvalErr<'tcx>>> {
let instance = ty::Instance::mono(cx.tcx, def_id);
let cid = GlobalId {
instance,
promoted: None
promoted: None,
};
let param_env = ty::ParamEnv::reveal_all();
let static_ = cx.tcx.const_eval(param_env.and(cid))?;
Expand Down Expand Up @@ -161,28 +161,19 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> {
bx: &Builder<'a, 'tcx>,
constant: &mir::Constant<'tcx>,
) -> Result<&'tcx ty::Const<'tcx>, Lrc<ConstEvalErr<'tcx>>> {
match constant.literal {
mir::Literal::Promoted { index } => {
let param_env = ty::ParamEnv::reveal_all();
let cid = mir::interpret::GlobalId {
instance: self.instance,
promoted: Some(index),
};
bx.tcx().const_eval(param_env.and(cid))
}
mir::Literal::Value { value } => {
Ok(self.monomorphize(&value))
}
}.and_then(|c| self.fully_evaluate(bx, c))
let c = self.monomorphize(&constant.literal);
self.fully_evaluate(bx, c)
}

/// process constant containing SIMD shuffle indices
pub fn simd_shuffle_indices(
&mut self,
bx: &Builder<'a, 'tcx>,
constant: &mir::Constant<'tcx>,
span: Span,
ty: Ty<'tcx>,
constant: Result<&'tcx ty::Const<'tcx>, Lrc<ConstEvalErr<'tcx>>>,
) -> (ValueRef, Ty<'tcx>) {
self.eval_mir_constant(bx, constant)
constant
.and_then(|c| {
let field_ty = c.ty.builtin_index().unwrap();
let fields = match c.ty.sty {
Expand Down Expand Up @@ -217,11 +208,11 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> {
})
.unwrap_or_else(|e| {
e.report_as_error(
bx.tcx().at(constant.span),
bx.tcx().at(span),
"could not evaluate shuffle_indices at compile time",
);
// We've errored, so we don't have to produce working code.
let ty = self.monomorphize(&constant.ty);
let ty = self.monomorphize(&ty);
let llty = bx.cx.layout_of(ty).llvm_type(bx.cx);
(C_undef(llty), ty)
})
Expand Down
37 changes: 9 additions & 28 deletions src/librustc_codegen_llvm/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use llvm::{ValueRef, LLVMConstInBoundsGEP};
use llvm::ValueRef;
use rustc::mir::interpret::ConstEvalErr;
use rustc::mir;
use rustc::mir::interpret::ConstValue;
Expand All @@ -22,14 +22,12 @@ use common::{CodegenCx, C_undef, C_usize};
use builder::{Builder, MemFlags};
use value::Value;
use type_of::LayoutLlvmExt;
use type_::Type;
use consts;

use std::fmt;
use std::ptr;

use super::{FunctionCx, LocalRef};
use super::constant::{scalar_to_llvm, const_alloc_to_llvm};
use super::constant::scalar_to_llvm;
use super::place::PlaceRef;

/// The representation of a Rust value. The enum variant is in fact
Expand Down Expand Up @@ -139,16 +137,7 @@ impl<'a, 'tcx> OperandRef<'tcx> {
OperandValue::Pair(a_llval, b_llval)
},
ConstValue::ByRef(alloc, offset) => {
let init = const_alloc_to_llvm(bx.cx, alloc);
let base_addr = consts::addr_of(bx.cx, init, layout.align, "byte_str");

let llval = unsafe { LLVMConstInBoundsGEP(
consts::bitcast(base_addr, Type::i8p(bx.cx)),
&C_usize(bx.cx, offset.bytes()),
1,
)};
let llval = consts::bitcast(llval, layout.llvm_type(bx.cx).ptr_to());
return Ok(PlaceRef::new_sized(llval, layout, alloc.align).load(bx));
return Ok(PlaceRef::from_const_alloc(bx, layout, alloc, offset).load(bx));
},
};

Expand Down Expand Up @@ -409,20 +398,12 @@ impl<'a, 'tcx> FunctionCx<'a, 'tcx> {
self.eval_mir_constant(bx, constant)
.and_then(|c| OperandRef::from_const(bx, c))
.unwrap_or_else(|err| {
match constant.literal {
mir::Literal::Promoted { .. } => {
// this is unreachable as long as runtime
// and compile-time agree on values
// With floats that won't always be true
// so we generate an abort below
},
mir::Literal::Value { .. } => {
err.report_as_error(
bx.tcx().at(constant.span),
"could not evaluate constant operand",
);
},
}
err.report_as_error(
bx.tcx().at(constant.span),
"could not evaluate constant operand",
);
// Allow RalfJ to sleep soundly knowing that even refactorings that remove
// the above error (or silence it under some conditions) will not cause UB
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️

let fnname = bx.cx.get_intrinsic(&("llvm.trap"));
bx.call(fnname, &[], None);
// We've errored, so we don't have to produce working code.
Expand Down
Loading