From 3d256b3ecbf340f752a78f653c37c5ff14834da9 Mon Sep 17 00:00:00 2001 From: SparrowLii Date: Thu, 21 Apr 2022 18:26:43 +0800 Subject: [PATCH 1/2] access `local_decls` through `ecx` --- .../rustc_mir_transform/src/const_prop.rs | 29 ++++------------ .../src/const_prop_lint.rs | 34 +++++-------------- 2 files changed, 14 insertions(+), 49 deletions(-) diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index 13b49256d488a..1efc9c37f21e7 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -12,9 +12,8 @@ use rustc_middle::mir::visit::{ MutVisitor, MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor, }; use rustc_middle::mir::{ - BasicBlock, BinOp, Body, Constant, ConstantKind, Local, LocalDecl, LocalKind, Location, - Operand, Place, Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, UnOp, - RETURN_PLACE, + BasicBlock, BinOp, Body, Constant, ConstantKind, Local, LocalKind, Location, Operand, Place, + Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, UnOp, RETURN_PLACE, }; use rustc_middle::ty::layout::{LayoutError, LayoutOf, LayoutOfHelpers, TyAndLayout}; use rustc_middle::ty::subst::{InternalSubsts, Subst}; @@ -313,9 +312,6 @@ struct ConstPropagator<'mir, 'tcx> { ecx: InterpCx<'mir, 'tcx, ConstPropMachine<'mir, 'tcx>>, tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, - // FIXME(eddyb) avoid cloning this field more than once, - // by accessing it through `ecx` instead. - local_decls: IndexVec>, // Because we have `MutVisitor` we can't obtain the `SourceInfo` from a `Location`. So we store // the last known `SourceInfo` here and just keep revisiting it. source_info: Option, @@ -361,10 +357,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { let substs = &InternalSubsts::identity_for_item(tcx, def_id); let param_env = tcx.param_env_reveal_all_normalized(def_id); - let span = tcx.def_span(def_id); - // FIXME: `CanConstProp::check` computes the layout of all locals, return those layouts - // so we can write them to `ecx.frame_mut().locals.layout, reducing the duplication in - // `layout_of` query invocations. let can_const_prop = CanConstProp::check(tcx, param_env, body); let mut only_propagate_inside_block_locals = BitSet::new_empty(can_const_prop.len()); for (l, mode) in can_const_prop.iter_enumerated() { @@ -374,7 +366,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { } let mut ecx = InterpCx::new( tcx, - span, + tcx.def_span(def_id), param_env, ConstPropMachine::new(only_propagate_inside_block_locals, can_const_prop), ); @@ -401,16 +393,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { ) .expect("failed to push initial stack frame"); - ConstPropagator { - ecx, - tcx, - param_env, - // FIXME(eddyb) avoid cloning this field more than once, - // by accessing it through `ecx` instead. - //FIXME(wesleywiser) we can't steal this because `Visitor::super_visit_body()` needs it - local_decls: body.local_decls.clone(), - source_info: None, - } + ConstPropagator { ecx, tcx, param_env, source_info: None } } fn get_const(&self, place: Place<'tcx>) -> Option> { @@ -511,7 +494,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { let r = r?; // We need the type of the LHS. We cannot use `place_layout` as that is the type // of the result, which for checked binops is not the same! - let left_ty = left.ty(&self.local_decls, self.tcx); + let left_ty = left.ty(&self.ecx.frame().body.local_decls, self.tcx); let left_size = self.ecx.layout_of(left_ty).ok()?.size; let right_size = r.layout.size; let r_bits = r.to_scalar().ok(); @@ -1133,7 +1116,7 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> { assert!( self.get_const(local.into()).is_none() || self - .layout_of(self.local_decls[local].ty) + .layout_of(self.ecx.frame().body.local_decls[local].ty) .map_or(true, |layout| layout.is_zst()) ) } diff --git a/compiler/rustc_mir_transform/src/const_prop_lint.rs b/compiler/rustc_mir_transform/src/const_prop_lint.rs index d6331a88c5b7d..2bd90998148e7 100644 --- a/compiler/rustc_mir_transform/src/const_prop_lint.rs +++ b/compiler/rustc_mir_transform/src/const_prop_lint.rs @@ -11,9 +11,9 @@ use rustc_index::bit_set::BitSet; use rustc_index::vec::IndexVec; use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor}; use rustc_middle::mir::{ - AssertKind, BasicBlock, BinOp, Body, Constant, ConstantKind, Local, LocalDecl, LocalKind, - Location, Operand, Place, Rvalue, SourceInfo, SourceScope, SourceScopeData, Statement, - StatementKind, Terminator, TerminatorKind, UnOp, RETURN_PLACE, + AssertKind, BasicBlock, BinOp, Body, Constant, ConstantKind, Local, LocalKind, Location, + Operand, Place, Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, UnOp, + RETURN_PLACE, }; use rustc_middle::ty::layout::{LayoutError, LayoutOf, LayoutOfHelpers, TyAndLayout}; use rustc_middle::ty::subst::{InternalSubsts, Subst}; @@ -308,10 +308,6 @@ struct ConstPropagator<'mir, 'tcx> { ecx: InterpCx<'mir, 'tcx, ConstPropMachine<'mir, 'tcx>>, tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, - // FIXME(eddyb) avoid cloning these two fields more than once, - // by accessing them through `ecx` instead. - source_scopes: IndexVec>, - local_decls: IndexVec>, // Because we have `MutVisitor` we can't obtain the `SourceInfo` from a `Location`. So we store // the last known `SourceInfo` here and just keep revisiting it. source_info: Option, @@ -357,10 +353,6 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { let substs = &InternalSubsts::identity_for_item(tcx, def_id); let param_env = tcx.param_env_reveal_all_normalized(def_id); - let span = tcx.def_span(def_id); - // FIXME: `CanConstProp::check` computes the layout of all locals, return those layouts - // so we can write them to `ecx.frame_mut().locals.layout, reducing the duplication in - // `layout_of` query invocations. let can_const_prop = CanConstProp::check(tcx, param_env, body); let mut only_propagate_inside_block_locals = BitSet::new_empty(can_const_prop.len()); for (l, mode) in can_const_prop.iter_enumerated() { @@ -370,7 +362,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { } let mut ecx = InterpCx::new( tcx, - span, + tcx.def_span(def_id), param_env, ConstPropMachine::new(only_propagate_inside_block_locals, can_const_prop), ); @@ -397,17 +389,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { ) .expect("failed to push initial stack frame"); - ConstPropagator { - ecx, - tcx, - param_env, - // FIXME(eddyb) avoid cloning these two fields more than once, - // by accessing them through `ecx` instead. - source_scopes: body.source_scopes.clone(), - //FIXME(wesleywiser) we can't steal this because `Visitor::super_visit_body()` needs it - local_decls: body.local_decls.clone(), - source_info: None, - } + ConstPropagator { ecx, tcx, param_env, source_info: None } } fn get_const(&self, place: Place<'tcx>) -> Option> { @@ -435,7 +417,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { } fn lint_root(&self, source_info: SourceInfo) -> Option { - source_info.scope.lint_root(&self.source_scopes) + source_info.scope.lint_root(&self.ecx.frame().body.source_scopes) } fn use_ecx(&mut self, f: F) -> Option @@ -572,7 +554,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { let r = r?; // We need the type of the LHS. We cannot use `place_layout` as that is the type // of the result, which for checked binops is not the same! - let left_ty = left.ty(&self.local_decls, self.tcx); + let left_ty = left.ty(&self.ecx.frame().body.local_decls, self.tcx); let left_size = self.ecx.layout_of(left_ty).ok()?.size; let right_size = r.layout.size; let r_bits = r.to_scalar().ok(); @@ -1026,7 +1008,7 @@ impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> { assert!( self.get_const(local.into()).is_none() || self - .layout_of(self.local_decls[local].ty) + .layout_of(self.ecx.frame().body.local_decls[local].ty) .map_or(true, |layout| layout.is_zst()) ) } From db23e773e326edccf38d8ddfd42310fc169b861d Mon Sep 17 00:00:00 2001 From: SparrowLii Date: Fri, 22 Apr 2022 08:42:38 +0800 Subject: [PATCH 2/2] use references to avoid function calls --- .../rustc_mir_transform/src/const_prop.rs | 18 +++++++++++---- .../src/const_prop_lint.rs | 23 +++++++++++++------ 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index 1efc9c37f21e7..691f4fb0e5425 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -12,8 +12,9 @@ use rustc_middle::mir::visit::{ MutVisitor, MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor, }; use rustc_middle::mir::{ - BasicBlock, BinOp, Body, Constant, ConstantKind, Local, LocalKind, Location, Operand, Place, - Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, UnOp, RETURN_PLACE, + BasicBlock, BinOp, Body, Constant, ConstantKind, Local, LocalDecl, LocalKind, Location, + Operand, Place, Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, UnOp, + RETURN_PLACE, }; use rustc_middle::ty::layout::{LayoutError, LayoutOf, LayoutOfHelpers, TyAndLayout}; use rustc_middle::ty::subst::{InternalSubsts, Subst}; @@ -312,6 +313,7 @@ struct ConstPropagator<'mir, 'tcx> { ecx: InterpCx<'mir, 'tcx, ConstPropMachine<'mir, 'tcx>>, tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, + local_decls: &'mir IndexVec>, // Because we have `MutVisitor` we can't obtain the `SourceInfo` from a `Location`. So we store // the last known `SourceInfo` here and just keep revisiting it. source_info: Option, @@ -393,7 +395,13 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { ) .expect("failed to push initial stack frame"); - ConstPropagator { ecx, tcx, param_env, source_info: None } + ConstPropagator { + ecx, + tcx, + param_env, + local_decls: &dummy_body.local_decls, + source_info: None, + } } fn get_const(&self, place: Place<'tcx>) -> Option> { @@ -494,7 +502,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { let r = r?; // We need the type of the LHS. We cannot use `place_layout` as that is the type // of the result, which for checked binops is not the same! - let left_ty = left.ty(&self.ecx.frame().body.local_decls, self.tcx); + let left_ty = left.ty(self.local_decls, self.tcx); let left_size = self.ecx.layout_of(left_ty).ok()?.size; let right_size = r.layout.size; let r_bits = r.to_scalar().ok(); @@ -1116,7 +1124,7 @@ impl<'tcx> MutVisitor<'tcx> for ConstPropagator<'_, 'tcx> { assert!( self.get_const(local.into()).is_none() || self - .layout_of(self.ecx.frame().body.local_decls[local].ty) + .layout_of(self.local_decls[local].ty) .map_or(true, |layout| layout.is_zst()) ) } diff --git a/compiler/rustc_mir_transform/src/const_prop_lint.rs b/compiler/rustc_mir_transform/src/const_prop_lint.rs index 2bd90998148e7..4945c10c9aaa9 100644 --- a/compiler/rustc_mir_transform/src/const_prop_lint.rs +++ b/compiler/rustc_mir_transform/src/const_prop_lint.rs @@ -11,9 +11,9 @@ use rustc_index::bit_set::BitSet; use rustc_index::vec::IndexVec; use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, PlaceContext, Visitor}; use rustc_middle::mir::{ - AssertKind, BasicBlock, BinOp, Body, Constant, ConstantKind, Local, LocalKind, Location, - Operand, Place, Rvalue, SourceInfo, Statement, StatementKind, Terminator, TerminatorKind, UnOp, - RETURN_PLACE, + AssertKind, BasicBlock, BinOp, Body, Constant, ConstantKind, Local, LocalDecl, LocalKind, + Location, Operand, Place, Rvalue, SourceInfo, SourceScope, SourceScopeData, Statement, + StatementKind, Terminator, TerminatorKind, UnOp, RETURN_PLACE, }; use rustc_middle::ty::layout::{LayoutError, LayoutOf, LayoutOfHelpers, TyAndLayout}; use rustc_middle::ty::subst::{InternalSubsts, Subst}; @@ -308,6 +308,8 @@ struct ConstPropagator<'mir, 'tcx> { ecx: InterpCx<'mir, 'tcx, ConstPropMachine<'mir, 'tcx>>, tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, + source_scopes: &'mir IndexVec>, + local_decls: &'mir IndexVec>, // Because we have `MutVisitor` we can't obtain the `SourceInfo` from a `Location`. So we store // the last known `SourceInfo` here and just keep revisiting it. source_info: Option, @@ -389,7 +391,14 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { ) .expect("failed to push initial stack frame"); - ConstPropagator { ecx, tcx, param_env, source_info: None } + ConstPropagator { + ecx, + tcx, + param_env, + source_scopes: &dummy_body.source_scopes, + local_decls: &dummy_body.local_decls, + source_info: None, + } } fn get_const(&self, place: Place<'tcx>) -> Option> { @@ -417,7 +426,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { } fn lint_root(&self, source_info: SourceInfo) -> Option { - source_info.scope.lint_root(&self.ecx.frame().body.source_scopes) + source_info.scope.lint_root(self.source_scopes) } fn use_ecx(&mut self, f: F) -> Option @@ -554,7 +563,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { let r = r?; // We need the type of the LHS. We cannot use `place_layout` as that is the type // of the result, which for checked binops is not the same! - let left_ty = left.ty(&self.ecx.frame().body.local_decls, self.tcx); + let left_ty = left.ty(self.local_decls, self.tcx); let left_size = self.ecx.layout_of(left_ty).ok()?.size; let right_size = r.layout.size; let r_bits = r.to_scalar().ok(); @@ -1008,7 +1017,7 @@ impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> { assert!( self.get_const(local.into()).is_none() || self - .layout_of(self.ecx.frame().body.local_decls[local].ty) + .layout_of(self.local_decls[local].ty) .map_or(true, |layout| layout.is_zst()) ) }