Skip to content

Optimize const_prop mir-opt by accessing local_decls through ecx #96281

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Apr 24, 2022
Merged

Optimize const_prop mir-opt by accessing local_decls through ecx #96281

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3d256b3
access `local_decls` through `ecx`
SparrowLii Apr 21, 2022
db23e77
use references to avoid function calls
SparrowLii Apr 22, 2022
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
17 changes: 4 additions & 13 deletions compiler/rustc_mir_transform/src/const_prop.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,9 +313,7 @@ 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<Local, LocalDecl<'tcx>>,
local_decls: &'mir IndexVec<Local, LocalDecl<'tcx>>,
// 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<SourceInfo>,
Expand Down Expand Up @@ -361,10 +359,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() {
Expand All @@ -374,7 +368,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),
);
Expand Down Expand Up @@ -405,10 +399,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
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(),
local_decls: &dummy_body.local_decls,
source_info: None,
}
}
Expand Down Expand Up @@ -511,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.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();
Expand Down
23 changes: 7 additions & 16 deletions compiler/rustc_mir_transform/src/const_prop_lint.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,10 +308,8 @@ 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<SourceScope, SourceScopeData<'tcx>>,
local_decls: IndexVec<Local, LocalDecl<'tcx>>,
source_scopes: &'mir IndexVec<SourceScope, SourceScopeData<'tcx>>,
local_decls: &'mir IndexVec<Local, LocalDecl<'tcx>>,
// 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<SourceInfo>,
Expand Down Expand Up @@ -357,10 +355,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() {
Expand All @@ -370,7 +364,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),
);
Expand Down Expand Up @@ -401,11 +395,8 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
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_scopes: &dummy_body.source_scopes,
local_decls: &dummy_body.local_decls,
source_info: None,
}
}
Expand Down Expand Up @@ -435,7 +426,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
}

fn lint_root(&self, source_info: SourceInfo) -> Option<HirId> {
source_info.scope.lint_root(&self.source_scopes)
source_info.scope.lint_root(self.source_scopes)
}

fn use_ecx<F, T>(&mut self, f: F) -> Option<T>
Expand Down Expand Up @@ -572,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.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();
Expand Down