Auto merge of #48052 - eddyb:deggregate, r=nikomatsakis

rustc_mir: handle all aggregate kinds in, and always run, the deaggregator.

This helps with removing`Rvalue::Aggregate` from the MIR, and with enabling more optimizations.
r? @nikomatsakis

src/librustc/lib.rs

@@ -69,6 +69,7 @@
 #![feature(underscore_lifetimes)]
 #![feature(universal_impl_trait)]
 #![feature(trace_macros)]
+#![feature(trusted_len)]
 #![feature(catch_expr)]
 #![feature(test)]
 

src/librustc/mir/mod.rs

@@ -34,13 +34,13 @@ use std::ascii;
 use std::borrow::{Cow};
 use std::cell::Ref;
 use std::fmt::{self, Debug, Formatter, Write};
-use std::{iter, u32};
+use std::{iter, mem, u32};
 use std::ops::{Index, IndexMut};
 use std::rc::Rc;
 use std::vec::IntoIter;
 use syntax::ast::{self, Name};
 use syntax::symbol::InternedString;
-use syntax_pos::Span;
+use syntax_pos::{Span, DUMMY_SP};
 
 mod cache;
 pub mod tcx;
@@ -984,11 +984,62 @@ impl<'tcx> BasicBlockData<'tcx> {
     pub fn retain_statements<F>(&mut self, mut f: F) where F: FnMut(&mut Statement) -> bool {
         for s in &mut self.statements {
             if !f(s) {
-                s.kind = StatementKind::Nop;
+                s.make_nop();
             }
         }
     }
 
+    pub fn expand_statements<F, I>(&mut self, mut f: F)
+        where F: FnMut(&mut Statement<'tcx>) -> Option<I>,
+              I: iter::TrustedLen<Item = Statement<'tcx>>
+    {
+        // Gather all the iterators we'll need to splice in, and their positions.
+        let mut splices: Vec<(usize, I)> = vec![];
+        let mut extra_stmts = 0;
+        for (i, s) in self.statements.iter_mut().enumerate() {
+            if let Some(mut new_stmts) = f(s) {
+                if let Some(first) = new_stmts.next() {
+                    // We can already store the first new statement.
+                    *s = first;
+
+                    // Save the other statements for optimized splicing.
+                    let remaining = new_stmts.size_hint().0;
+                    if remaining > 0 {
+                        splices.push((i + 1 + extra_stmts, new_stmts));
+                        extra_stmts += remaining;
+                    }
+                } else {
+                    s.make_nop();
+                }
+            }
+        }
+
+        // Splice in the new statements, from the end of the block.
+        // FIXME(eddyb) This could be more efficient with a "gap buffer"
+        // where a range of elements ("gap") is left uninitialized, with
+        // splicing adding new elements to the end of that gap and moving
+        // existing elements from before the gap to the end of the gap.
+        // For now, this is safe code, emulating a gap but initializing it.
+        let mut gap = self.statements.len()..self.statements.len()+extra_stmts;
+        self.statements.resize(gap.end, Statement {
+            source_info: SourceInfo {
+                span: DUMMY_SP,
+                scope: ARGUMENT_VISIBILITY_SCOPE
+            },
+            kind: StatementKind::Nop
+        });
+        for (splice_start, new_stmts) in splices.into_iter().rev() {
+            let splice_end = splice_start + new_stmts.size_hint().0;
+            while gap.end > splice_end {
+                gap.start -= 1;
+                gap.end -= 1;
+                self.statements.swap(gap.start, gap.end);
+            }
+            self.statements.splice(splice_start..splice_end, new_stmts);
+            gap.end = splice_start;
+        }
+    }
+
     pub fn visitable(&self, index: usize) -> &dyn MirVisitable<'tcx> {
         if index < self.statements.len() {
             &self.statements[index]
@@ -1157,6 +1208,14 @@ impl<'tcx> Statement<'tcx> {
     pub fn make_nop(&mut self) {
         self.kind = StatementKind::Nop
     }
+
+    /// Changes a statement to a nop and returns the original statement.
+    pub fn replace_nop(&mut self) -> Self {
+        Statement {
+            source_info: self.source_info,
+            kind: mem::replace(&mut self.kind, StatementKind::Nop)
+        }
+    }
 }
 
 #[derive(Clone, Debug, RustcEncodable, RustcDecodable)]

src/librustc_mir/transform/deaggregator.rs

@@ -21,116 +21,90 @@ impl MirPass for Deaggregator {
                           tcx: TyCtxt<'a, 'tcx, 'tcx>,
                           source: MirSource,
                           mir: &mut Mir<'tcx>) {
-        let node_path = tcx.item_path_str(source.def_id);
-        debug!("running on: {:?}", node_path);
-        // we only run when mir_opt_level > 2
-        if tcx.sess.opts.debugging_opts.mir_opt_level <= 2 {
-            return;
-        }
-
         // Don't run on constant MIR, because trans might not be able to
         // evaluate the modified MIR.
         // FIXME(eddyb) Remove check after miri is merged.
         let id = tcx.hir.as_local_node_id(source.def_id).unwrap();
         match (tcx.hir.body_owner_kind(id), source.promoted) {
-            (hir::BodyOwnerKind::Fn, None) => {},
+            (_, Some(_)) |
-            _ => return
+            (hir::BodyOwnerKind::Const, _) |
+            (hir::BodyOwnerKind::Static(_), _) => return,
+
+            (hir::BodyOwnerKind::Fn, _) => {
+                if tcx.is_const_fn(source.def_id) {
+                    // Don't run on const functions, as, again, trans might not be able to evaluate
+                    // the optimized IR.
+                    return
+                }
+            }
         }
-        // In fact, we might not want to trigger in other cases.
-        // Ex: when we could use SROA.  See issue #35259
 
-        for bb in mir.basic_blocks_mut() {
+        let (basic_blocks, local_decls) = mir.basic_blocks_and_local_decls_mut();
-            let mut curr: usize = 0;
+        let local_decls = &*local_decls;
-            while let Some(idx) = get_aggregate_statement_index(curr, &bb.statements) {
+        for bb in basic_blocks {
-                // do the replacement
+            bb.expand_statements(|stmt| {
-                debug!("removing statement {:?}", idx);
+                // FIXME(eddyb) don't match twice on `stmt.kind` (post-NLL).
-                let src_info = bb.statements[idx].source_info;
+                if let StatementKind::Assign(_, ref rhs) = stmt.kind {
-                let suffix_stmts = bb.statements.split_off(idx+1);
+                    if let Rvalue::Aggregate(ref kind, _) = *rhs {
-                let orig_stmt = bb.statements.pop().unwrap();
+                        // FIXME(#48193) Deaggregate arrays when it's cheaper to do so.
-                let (lhs, rhs) = match orig_stmt.kind {
+                        if let AggregateKind::Array(_) = **kind {
-                    StatementKind::Assign(ref lhs, ref rhs) => (lhs, rhs),
+                            return None;
-                    _ => span_bug!(src_info.span, "expected assign, not {:?}", orig_stmt),
+                        }
-                };
-                let (agg_kind, operands) = match rhs {
-                    &Rvalue::Aggregate(ref agg_kind, ref operands) => (agg_kind, operands),
-                    _ => span_bug!(src_info.span, "expected aggregate, not {:?}", rhs),
-                };
-                let (adt_def, variant, substs) = match **agg_kind {
-                    AggregateKind::Adt(adt_def, variant, substs, None)
-                        => (adt_def, variant, substs),
-                    _ => span_bug!(src_info.span, "expected struct, not {:?}", rhs),
-                };
-                let n = bb.statements.len();
-                bb.statements.reserve(n + operands.len() + suffix_stmts.len());
-                for (i, op) in operands.iter().enumerate() {
-                    let ref variant_def = adt_def.variants[variant];
-                    let ty = variant_def.fields[i].ty(tcx, substs);
-                    let rhs = Rvalue::Use(op.clone());
-
-                    let lhs_cast = if adt_def.is_enum() {
-                        Place::Projection(Box::new(PlaceProjection {
-                            base: lhs.clone(),
-                            elem: ProjectionElem::Downcast(adt_def, variant),
-                        }))
                     } else {
-                        lhs.clone()
+                        return None;
-                    };
+                    }
-
+                } else {
-                    let lhs_proj = Place::Projection(Box::new(PlaceProjection {
+                    return None;
-                        base: lhs_cast,
-                        elem: ProjectionElem::Field(Field::new(i), ty),
-                    }));
-                    let new_statement = Statement {
-                        source_info: src_info,
-                        kind: StatementKind::Assign(lhs_proj, rhs),
-                    };
-                    debug!("inserting: {:?} @ {:?}", new_statement, idx + i);
-                    bb.statements.push(new_statement);
                 }
 
-                // if the aggregate was an enum, we need to set the discriminant
+                let stmt = stmt.replace_nop();
-                if adt_def.is_enum() {
+                let source_info = stmt.source_info;
-                    let set_discriminant = Statement {
+                let (mut lhs, kind, operands) = match stmt.kind {
-                        kind: StatementKind::SetDiscriminant {
+                    StatementKind::Assign(lhs, Rvalue::Aggregate(kind, operands))
-                            place: lhs.clone(),
+                        => (lhs, kind, operands),
-                            variant_index: variant,
+                    _ => bug!()
-                        },
-                        source_info: src_info,
-                    };
-                    bb.statements.push(set_discriminant);
                 };
 
-                curr = bb.statements.len();
+                let mut set_discriminant = None;
-                bb.statements.extend(suffix_stmts);
+                let active_field_index = match *kind {
-            }
+                    AggregateKind::Adt(adt_def, variant_index, _, active_field_index) => {
+                        if adt_def.is_enum() {
+                            set_discriminant = Some(Statement {
+                                kind: StatementKind::SetDiscriminant {
+                                    place: lhs.clone(),
+                                    variant_index,
+                                },
+                                source_info,
+                            });
+                            lhs = lhs.downcast(adt_def, variant_index);
+                        }
+                        active_field_index
+                    }
+                    _ => None
+                };
+
+                Some(operands.into_iter().enumerate().map(move |(i, op)| {
+                    let lhs_field = if let AggregateKind::Array(_) = *kind {
+                        // FIXME(eddyb) `offset` should be u64.
+                        let offset = i as u32;
+                        assert_eq!(offset as usize, i);
+                        lhs.clone().elem(ProjectionElem::ConstantIndex {
+                            offset,
+                            // FIXME(eddyb) `min_length` doesn't appear to be used.
+                            min_length: offset + 1,
+                            from_end: false
+                        })
+                    } else {
+                        let ty = op.ty(local_decls, tcx);
+                        let field = Field::new(active_field_index.unwrap_or(i));
+                        lhs.clone().field(field, ty)
+                    };
+                    Statement {
+                        source_info,
+                        kind: StatementKind::Assign(lhs_field, Rvalue::Use(op)),
+                    }
+                }).chain(set_discriminant))
+            });
         }
     }
 }
-
-fn get_aggregate_statement_index<'a, 'tcx, 'b>(start: usize,
-                                         statements: &Vec<Statement<'tcx>>)
-                                         -> Option<usize> {
-    for i in start..statements.len() {
-        let ref statement = statements[i];
-        let rhs = match statement.kind {
-            StatementKind::Assign(_, ref rhs) => rhs,
-            _ => continue,
-        };
-        let (kind, operands) = match rhs {
-            &Rvalue::Aggregate(ref kind, ref operands) => (kind, operands),
-            _ => continue,
-        };
-        let (adt_def, variant) = match **kind {
-            AggregateKind::Adt(adt_def, variant, _, None) => (adt_def, variant),
-            _ => continue,
-        };
-        if operands.len() == 0 {
-            // don't deaggregate ()
-            continue;
-        }
-        debug!("getting variant {:?}", variant);
-        debug!("for adt_def {:?}", adt_def);
-        return Some(i);
-    };
-    None
-}

src/librustc_mir/transform/mod.rs

@@ -258,6 +258,11 @@ fn optimized_mir<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx
 
         // Optimizations begin.
         inline::Inline,
+
+        // Lowering generator control-flow and variables
+        // has to happen before we do anything else to them.
+        generator::StateTransform,
+
         instcombine::InstCombine,
         deaggregator::Deaggregator,
         copy_prop::CopyPropagation,
@@ -265,7 +270,6 @@ fn optimized_mir<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> &'tcx
         simplify::SimplifyCfg::new("final"),
         simplify::SimplifyLocals,
 
-        generator::StateTransform,
         add_call_guards::CriticalCallEdges,
         dump_mir::Marker("PreTrans"),
     ];

src/librustc_mir/transform/simplify.rs

@@ -42,6 +42,7 @@ use rustc_data_structures::indexed_vec::{Idx, IndexVec};
 use rustc::ty::TyCtxt;
 use rustc::mir::*;
 use rustc::mir::visit::{MutVisitor, Visitor, PlaceContext};
+use rustc::session::config::FullDebugInfo;
 use std::borrow::Cow;
 use transform::{MirPass, MirSource};
 
@@ -281,16 +282,24 @@ pub struct SimplifyLocals;
 
 impl MirPass for SimplifyLocals {
     fn run_pass<'a, 'tcx>(&self,
-                          _: TyCtxt<'a, 'tcx, 'tcx>,
+                          tcx: TyCtxt<'a, 'tcx, 'tcx>,
                           _: MirSource,
                           mir: &mut Mir<'tcx>) {
         let mut marker = DeclMarker { locals: BitVector::new(mir.local_decls.len()) };
         marker.visit_mir(mir);
         // Return pointer and arguments are always live
-        marker.locals.insert(0);
+        marker.locals.insert(RETURN_PLACE.index());
-        for idx in mir.args_iter() {
+        for arg in mir.args_iter() {
-            marker.locals.insert(idx.index());
+            marker.locals.insert(arg.index());
         }
+
+        // We may need to keep dead user variables live for debuginfo.
+        if tcx.sess.opts.debuginfo == FullDebugInfo {
+            for local in mir.vars_iter() {
+                marker.locals.insert(local.index());
+            }
+        }
+
         let map = make_local_map(&mut mir.local_decls, marker.locals);
         // Update references to all vars and tmps now
         LocalUpdater { map: map }.visit_mir(mir);

src/test/codegen/lifetime_start_end.rs

@@ -28,14 +28,14 @@ pub fn test() {
 // CHECK: [[S_b:%[0-9]+]] = bitcast %"core::option::Option<i32>"** %b to i8*
 // CHECK: call void @llvm.lifetime.start{{.*}}(i{{[0-9 ]+}}, i8* [[S_b]])
 
-// CHECK: [[S__5:%[0-9]+]] = bitcast %"core::option::Option<i32>"* %_5 to i8*
+// CHECK: [[S__4:%[0-9]+]] = bitcast %"core::option::Option<i32>"* %_4 to i8*
-// CHECK: call void @llvm.lifetime.start{{.*}}(i{{[0-9 ]+}}, i8* [[S__5]])
+// CHECK: call void @llvm.lifetime.start{{.*}}(i{{[0-9 ]+}}, i8* [[S__4]])
 
 // CHECK: [[E_b:%[0-9]+]] = bitcast %"core::option::Option<i32>"** %b to i8*
 // CHECK: call void @llvm.lifetime.end{{.*}}(i{{[0-9 ]+}}, i8* [[E_b]])
 
-// CHECK: [[E__5:%[0-9]+]] = bitcast %"core::option::Option<i32>"* %_5 to i8*
+// CHECK: [[E__4:%[0-9]+]] = bitcast %"core::option::Option<i32>"* %_4 to i8*
-// CHECK: call void @llvm.lifetime.end{{.*}}(i{{[0-9 ]+}}, i8* [[E__5]])
+// CHECK: call void @llvm.lifetime.end{{.*}}(i{{[0-9 ]+}}, i8* [[E__4]])
     }
 
     let c = 1;

src/test/codegen/match.rs

@@ -19,7 +19,7 @@ pub enum E {
 
 // CHECK-LABEL: @exhaustive_match
 #[no_mangle]
-pub fn exhaustive_match(e: E) {
+pub fn exhaustive_match(e: E, unit: ()) {
 // CHECK: switch{{.*}}, label %[[OTHERWISE:[a-zA-Z0-9_]+]] [
 // CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[A:[a-zA-Z0-9_]+]]
 // CHECK-NEXT: i[[TY:[0-9]+]] [[DISCR:[0-9]+]], label %[[B:[a-zA-Z0-9_]+]]
@@ -31,7 +31,7 @@ pub fn exhaustive_match(e: E) {
 // CHECK: [[OTHERWISE]]:
 // CHECK-NEXT: unreachable
     match e {
-        E::A => (),
+        E::A => unit,
-        E::B => (),
+        E::B => unit,
     }
 }

src/test/incremental/hashes/closure_expressions.rs

@@ -64,7 +64,7 @@ pub fn change_parameter_pattern() {
 }
 
 #[cfg(not(cfail1))]
-#[rustc_clean(cfg="cfail2", except="HirBody, MirValidated, MirOptimized, TypeckTables")]
+#[rustc_clean(cfg="cfail2", except="HirBody, MirValidated, TypeckTables")]
 #[rustc_clean(cfg="cfail3")]
 pub fn change_parameter_pattern() {
     let _ = |&x: &u32| x;

src/test/incremental/issue-38222.rs

@@ -18,7 +18,7 @@
 #![feature(rustc_attrs)]
 
 
-#![rustc_partition_translated(module="issue_38222-mod1", cfg="rpass2")]
+#![rustc_partition_reused(module="issue_38222-mod1", cfg="rpass2")]
 
 // If trans had added a dependency edge to the Krate dep-node, nothing would
 // be re-used, so checking that this module was re-used is sufficient.

src/test/mir-opt/copy_propagation_arg.rs

@@ -78,7 +78,7 @@ fn main() {
 // bb1: {
 //     StorageDead(_3);
 //     _1 = const 5u8;
-//     _0 = ();
+//     ...
 //     return;
 // }
 // END rustc.bar.CopyPropagation.before.mir
@@ -92,6 +92,7 @@ fn main() {
 //     ...
 //     _1 = const 5u8;
 //     ...
+//     return;
 // }
 // END rustc.bar.CopyPropagation.after.mir
 // START rustc.baz.CopyPropagation.before.mir
@@ -100,7 +101,7 @@ fn main() {
 //     _2 = _1;
 //     _1 = move _2;
 //     StorageDead(_2);
-//     _0 = ();
+//     ...
 //     return;
 // }
 // END rustc.baz.CopyPropagation.before.mir
@@ -110,21 +111,22 @@ fn main() {
 //     _2 = _1;
 //     _1 = move _2;
 //     ...
+//     return;
 // }
 // END rustc.baz.CopyPropagation.after.mir
 // START rustc.arg_src.CopyPropagation.before.mir
 // bb0: {
-//       ...
+//      ...
-//       _3 = _1;
+//      _3 = _1;
-//       _2 = move _3;
+//      _2 = move _3;
-//       ...
+//      ...
-//       _1 = const 123i32;
+//      _1 = const 123i32;
-//       ...
+//      ...
-//       _4 = _2;
+//      _4 = _2;
-//       _0 = move _4;
+//      _0 = move _4;
-//       ...
+//      ...
-//       return;
+//      return;
-//   }
+//  }
 // END rustc.arg_src.CopyPropagation.before.mir
 // START rustc.arg_src.CopyPropagation.after.mir
 // bb0: {

src/test/run-make/sepcomp-cci-copies/Makefile

@@ -2,9 +2,11 @@
 
 # Check that cross-crate inlined items are inlined in all compilation units
 # that refer to them, and not in any other compilation units.
+# Note that we have to pass `-C codegen-units=6` because up to two CGUs may be
+# created for each source module (see `rustc_mir::monomorphize::partitioning`).
 
 all:
 	$(RUSTC) cci_lib.rs
-	$(RUSTC) foo.rs --emit=llvm-ir -C codegen-units=3 \
+	$(RUSTC) foo.rs --emit=llvm-ir -C codegen-units=6 \
 		-Z inline-in-all-cgus
 	[ "$$(cat "$(TMPDIR)"/foo.*.ll | grep -c define\ .*cci_fn)" -eq "2" ]

src/test/ui/print_type_sizes/generics.rs

@@ -72,7 +72,7 @@ pub fn f1<T:Copy>(x: T) {
 fn start(_: isize, _: *const *const u8) -> isize {
     let _b: Pair<u8> = Pair::new(0, 0);
     let _s: Pair<SevenBytes> = Pair::new(SevenBytes::new(), SevenBytes::new());
-    let _z: ZeroSized = ZeroSized;
+    let ref _z: ZeroSized = ZeroSized;
     f1::<SevenBytes>(SevenBytes::new());
     0
 }