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

Reduce size of hir::Expr by boxing more of hir::InlineAsm #66515

Merged
merged 1 commit into from
Nov 21, 2019
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
7 changes: 3 additions & 4 deletions src/librustc/hir/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1086,10 +1086,9 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
ExprKind::Ret(ref optional_expression) => {
walk_list!(visitor, visit_expr, optional_expression);
}
ExprKind::InlineAsm(_, ref outputs, ref inputs) => {
for expr in outputs.iter().chain(inputs.iter()) {
visitor.visit_expr(expr)
}
ExprKind::InlineAsm(ref asm) => {
walk_list!(visitor, visit_expr, &asm.outputs_exprs);
walk_list!(visitor, visit_expr, &asm.inputs_exprs);
}
ExprKind::Yield(ref subexpression, _) => {
visitor.visit_expr(subexpression);
Expand Down
26 changes: 13 additions & 13 deletions src/librustc/hir/lowering/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -966,7 +966,7 @@ impl LoweringContext<'_> {
}

fn lower_expr_asm(&mut self, asm: &InlineAsm) -> hir::ExprKind {
let hir_asm = hir::InlineAsm {
let inner = hir::InlineAsmInner {
inputs: asm.inputs.iter().map(|&(ref c, _)| c.clone()).collect(),
outputs: asm.outputs
.iter()
Expand All @@ -984,18 +984,18 @@ impl LoweringContext<'_> {
alignstack: asm.alignstack,
dialect: asm.dialect,
};

let outputs = asm.outputs
.iter()
.map(|out| self.lower_expr(&out.expr))
.collect();

let inputs = asm.inputs
.iter()
.map(|&(_, ref input)| self.lower_expr(input))
.collect();

hir::ExprKind::InlineAsm(P(hir_asm), outputs, inputs)
let hir_asm = hir::InlineAsm {
inner,
inputs_exprs: asm.inputs
.iter()
.map(|&(_, ref input)| self.lower_expr(input))
.collect(),
outputs_exprs: asm.outputs
.iter()
.map(|out| self.lower_expr(&out.expr))
.collect(),
};
hir::ExprKind::InlineAsm(P(hir_asm))
}

fn lower_field(&mut self, f: &Field) -> hir::Field {
Expand Down
13 changes: 10 additions & 3 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1457,7 +1457,7 @@ pub struct Expr {

// `Expr` is used a lot. Make sure it doesn't unintentionally get bigger.
#[cfg(target_arch = "x86_64")]
static_assert_size!(Expr, 72);
static_assert_size!(Expr, 64);

impl Expr {
pub fn precedence(&self) -> ExprPrecedence {
Expand Down Expand Up @@ -1656,7 +1656,7 @@ pub enum ExprKind {
Ret(Option<P<Expr>>),

/// Inline assembly (from `asm!`), with its outputs and inputs.
InlineAsm(P<InlineAsm>, HirVec<Expr>, HirVec<Expr>),
InlineAsm(P<InlineAsm>),

/// A struct or struct-like variant literal expression.
///
Expand Down Expand Up @@ -2063,7 +2063,7 @@ pub struct InlineAsmOutput {
// NOTE(eddyb) This is used within MIR as well, so unlike the rest of the HIR,
// it needs to be `Clone` and use plain `Vec<T>` instead of `HirVec<T>`.
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
#[derive(Clone, RustcEncodable, RustcDecodable, Debug, HashStable)]
pub struct InlineAsm {
pub struct InlineAsmInner {
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
pub asm: Symbol,
pub asm_str_style: StrStyle,
pub outputs: Vec<InlineAsmOutput>,
Expand All @@ -2074,6 +2074,13 @@ pub struct InlineAsm {
pub dialect: AsmDialect,
}

#[derive(RustcEncodable, RustcDecodable, Debug, HashStable)]
pub struct InlineAsm {
pub inner: InlineAsmInner,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any need to have InlineAsmInner? Could we just have asm, asm_str_style, outputs, and dialect fields directly in InlineAsm?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried but wasn't able to do that; Expr is not Clone and InlineAsmInner needs to be Clone.

pub outputs_exprs: HirVec<Expr>,
pub inputs_exprs: HirVec<Expr>,
}

/// Represents a parameter in a function header.
#[derive(RustcEncodable, RustcDecodable, Debug, HashStable)]
pub struct Param {
Expand Down
21 changes: 11 additions & 10 deletions src/librustc/hir/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1365,14 +1365,15 @@ impl<'a> State<'a> {
self.print_expr_maybe_paren(&expr, parser::PREC_JUMP);
}
}
hir::ExprKind::InlineAsm(ref a, ref outputs, ref inputs) => {
hir::ExprKind::InlineAsm(ref a) => {
let i = &a.inner;
self.s.word("asm!");
self.popen();
self.print_string(&a.asm.as_str(), a.asm_str_style);
self.print_string(&i.asm.as_str(), i.asm_str_style);
self.word_space(":");

let mut out_idx = 0;
self.commasep(Inconsistent, &a.outputs, |s, out| {
self.commasep(Inconsistent, &i.outputs, |s, out| {
let constraint = out.constraint.as_str();
let mut ch = constraint.chars();
match ch.next() {
Expand All @@ -1383,36 +1384,36 @@ impl<'a> State<'a> {
_ => s.print_string(&constraint, ast::StrStyle::Cooked),
}
s.popen();
s.print_expr(&outputs[out_idx]);
s.print_expr(&a.outputs_exprs[out_idx]);
s.pclose();
out_idx += 1;
});
self.s.space();
self.word_space(":");

let mut in_idx = 0;
self.commasep(Inconsistent, &a.inputs, |s, co| {
self.commasep(Inconsistent, &i.inputs, |s, co| {
s.print_string(&co.as_str(), ast::StrStyle::Cooked);
s.popen();
s.print_expr(&inputs[in_idx]);
s.print_expr(&a.inputs_exprs[in_idx]);
s.pclose();
in_idx += 1;
});
self.s.space();
self.word_space(":");

self.commasep(Inconsistent, &a.clobbers, |s, co| {
self.commasep(Inconsistent, &i.clobbers, |s, co| {
s.print_string(&co.as_str(), ast::StrStyle::Cooked);
});

let mut options = vec![];
if a.volatile {
if i.volatile {
options.push("volatile");
}
if a.alignstack {
if i.alignstack {
options.push("alignstack");
}
if a.dialect == ast::AsmDialect::Intel {
if i.dialect == ast::AsmDialect::Intel {
options.push("intel");
}

Expand Down
6 changes: 3 additions & 3 deletions src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -283,15 +283,15 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
self.borrow_expr(&base, bk);
}

hir::ExprKind::InlineAsm(ref ia, ref outputs, ref inputs) => {
for (o, output) in ia.outputs.iter().zip(outputs) {
hir::ExprKind::InlineAsm(ref ia) => {
for (o, output) in ia.inner.outputs.iter().zip(&ia.outputs_exprs) {
if o.is_indirect {
self.consume_expr(output);
} else {
self.mutate_expr(output);
}
}
self.consume_exprs(inputs);
self.consume_exprs(&ia.inputs_exprs);
}

hir::ExprKind::Continue(..) |
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

use crate::hir::def::{CtorKind, Namespace};
use crate::hir::def_id::DefId;
use crate::hir::{self, InlineAsm as HirInlineAsm};
use crate::hir;
use crate::mir::interpret::{PanicInfo, Scalar};
use crate::mir::visit::MirVisitable;
use crate::ty::adjustment::PointerCast;
Expand Down Expand Up @@ -1638,7 +1638,7 @@ pub enum FakeReadCause {

#[derive(Clone, Debug, RustcEncodable, RustcDecodable, HashStable, TypeFoldable)]
pub struct InlineAsm<'tcx> {
pub asm: HirInlineAsm,
pub asm: hir::InlineAsmInner,
pub outputs: Box<[Place<'tcx>]>,
pub inputs: Box<[(Span, Operand<'tcx>)]>,
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ CloneTypeFoldableAndLiftImpls! {
::syntax_pos::symbol::Symbol,
crate::hir::def::Res,
crate::hir::def_id::DefId,
crate::hir::InlineAsm,
crate::hir::InlineAsmInner,
crate::hir::MatchSource,
crate::hir::Mutability,
crate::hir::Unsafety,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use libc::{c_uint, c_char};
impl AsmBuilderMethods<'tcx> for Builder<'a, 'll, 'tcx> {
fn codegen_inline_asm(
&mut self,
ia: &hir::InlineAsm,
ia: &hir::InlineAsmInner,
outputs: Vec<PlaceRef<'tcx, &'ll Value>>,
mut inputs: Vec<&'ll Value>,
span: Span,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_codegen_ssa/traits/asm.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
use super::BackendTypes;
use crate::mir::place::PlaceRef;
use rustc::hir::{GlobalAsm, InlineAsm};
use rustc::hir::{GlobalAsm, InlineAsmInner};
use syntax_pos::Span;

pub trait AsmBuilderMethods<'tcx>: BackendTypes {
/// Take an inline assembly expression and splat it out via LLVM
fn codegen_inline_asm(
&mut self,
ia: &InlineAsm,
ia: &InlineAsmInner,
outputs: Vec<PlaceRef<'tcx, Self::Value>>,
inputs: Vec<Self::Value>,
span: Span,
Expand Down
8 changes: 4 additions & 4 deletions src/librustc_mir/hair/cx/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -533,11 +533,11 @@ fn make_mirror_unadjusted<'a, 'tcx>(
convert_path_expr(cx, expr, res)
}

hir::ExprKind::InlineAsm(ref asm, ref outputs, ref inputs) => {
hir::ExprKind::InlineAsm(ref asm) => {
ExprKind::InlineAsm {
asm,
outputs: outputs.to_ref(),
inputs: inputs.to_ref(),
asm: &asm.inner,
outputs: asm.outputs_exprs.to_ref(),
inputs: asm.inputs_exprs.to_ref(),
}
}

Expand Down
6 changes: 5 additions & 1 deletion src/librustc_mir/hair/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,10 @@ pub enum StmtKind<'tcx> {
},
}

// `Expr` is used a lot. Make sure it doesn't unintentionally get bigger.
#[cfg(target_arch = "x86_64")]
rustc_data_structures::static_assert_size!(Expr<'_>, 168);

/// The Hair trait implementor lowers their expressions (`&'tcx H::Expr`)
/// into instances of this `Expr` enum. This lowering can be done
/// basically as lazily or as eagerly as desired: every recursive
Expand Down Expand Up @@ -264,7 +268,7 @@ pub enum ExprKind<'tcx> {
user_ty: Option<Canonical<'tcx, UserType<'tcx>>>,
},
InlineAsm {
asm: &'tcx hir::InlineAsm,
asm: &'tcx hir::InlineAsmInner,
outputs: Vec<ExprRef<'tcx>>,
inputs: Vec<ExprRef<'tcx>>
},
Expand Down
30 changes: 17 additions & 13 deletions src/librustc_passes/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1184,17 +1184,21 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
self.propagate_through_expr(&e, succ)
}

hir::ExprKind::InlineAsm(ref ia, ref outputs, ref inputs) => {
hir::ExprKind::InlineAsm(ref asm) => {
let ia = &asm.inner;
let outputs = &asm.outputs_exprs;
let inputs = &asm.inputs_exprs;
let succ = ia.outputs.iter().zip(outputs).rev().fold(succ, |succ, (o, output)| {
// see comment on places
// in propagate_through_place_components()
if o.is_indirect {
self.propagate_through_expr(output, succ)
} else {
let acc = if o.is_rw { ACC_WRITE|ACC_READ } else { ACC_WRITE };
let succ = self.write_place(output, succ, acc);
self.propagate_through_place_components(output, succ)
}});
// see comment on places
// in propagate_through_place_components()
if o.is_indirect {
self.propagate_through_expr(output, succ)
} else {
let acc = if o.is_rw { ACC_WRITE|ACC_READ } else { ACC_WRITE };
let succ = self.write_place(output, succ, acc);
self.propagate_through_place_components(output, succ)
}
});

// Inputs are executed first. Propagate last because of rev order
self.propagate_through_exprs(inputs, succ)
Expand Down Expand Up @@ -1395,13 +1399,13 @@ fn check_expr<'tcx>(this: &mut Liveness<'_, 'tcx>, expr: &'tcx Expr) {
}
}

hir::ExprKind::InlineAsm(ref ia, ref outputs, ref inputs) => {
for input in inputs {
hir::ExprKind::InlineAsm(ref asm) => {
for input in &asm.inputs_exprs {
this.visit_expr(input);
}

// Output operands must be places
for (o, output) in ia.outputs.iter().zip(outputs) {
for (o, output) in asm.inner.outputs.iter().zip(&asm.outputs_exprs) {
if !o.is_indirect {
this.check_place(output);
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_typeck/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
ExprKind::Path(ref qpath) => {
self.check_expr_path(qpath, expr)
}
ExprKind::InlineAsm(_, ref outputs, ref inputs) => {
for expr in outputs.iter().chain(inputs.iter()) {
ExprKind::InlineAsm(ref asm) => {
for expr in asm.outputs_exprs.iter().chain(asm.inputs_exprs.iter()) {
self.check_expr(expr);
}
tcx.mk_unit()
Expand Down