Skip to content

Commit 5168a8a

Browse files
b-naberb-naber
committed
add NodeKind::Local
1 parent 21a9b51 commit 5168a8a

File tree

1 file changed

+105
-53
lines changed

1 file changed

+105
-53
lines changed

‎compiler/rustc_mir_dataflow/src/impls/live_borrows.rs

Lines changed: 105 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -26,22 +26,25 @@ use either::Either;
2626
2727
#[derive(Copy, Clone, Debug)]
2828
enum NodeKind {
29-
// An node corresponding to the place on the lhs of a statement like
30-
// `_3 = Ref(_, _, _4)`
29+
// An node corresponding to the place of the borrowed place (`_4` in this case) in
30+
// an assignment like `_3 = Ref(_, _, _4)`.
3131
Borrow(Local),
3232

3333
// Nodes corresponding to the place on the lhs of a statement like
3434
// `_2 = Aggregate(Adt(..), _, _, _, _), [move _3, move _6])`,
35-
// where _3 and _6 are places corresponding to references or raw pointers
36-
// or locals that are borrowed (`_2 = Ref(..)`).
37-
LocalOrLocalWithRefs(Local),
35+
// where _3 and _6 are places corresponding to references or raw pointers.
36+
LocalWithRefs(Local),
37+
38+
// Nodes corresponding to the place on the lhs of an assignment like `_2 = Ref(..)`.
39+
Local(Local),
3840
}
3941

4042
impl NodeKind {
4143
fn get_local(&self) -> Local {
4244
match self {
4345
Self::Borrow(local) => *local,
44-
Self::LocalOrLocalWithRefs(local) => *local,
46+
Self::LocalWithRefs(local) => *local,
47+
Self::Local(local) => *local,
4548
}
4649
}
4750
}
@@ -103,9 +106,8 @@ struct BorrowDependencies<'a, 'tcx> {
103106
// liveness analysis.
104107
dep_graph: Graph<NodeKind, ()>,
105108

106-
// Contains the `NodeIndex` corresponding to the local to which we're currently
107-
// assigning.
108-
current_local: Option<NodeIndex>,
109+
// Contains the `Local` to which we're currently assigning.
110+
current_local: Option<Local>,
109111
}
110112

111113
impl<'a, 'tcx> BorrowDependencies<'a, 'tcx> {
@@ -122,6 +124,17 @@ impl<'a, 'tcx> BorrowDependencies<'a, 'tcx> {
122124
locals_to_node_indexes: Default::default(),
123125
}
124126
}
127+
128+
fn maybe_create_node(&mut self, node_kind: NodeKind) -> NodeIndex {
129+
let local = node_kind.get_local();
130+
if let Some(node_idx) = self.locals_to_node_indexes.get(&local) {
131+
*node_idx
132+
} else {
133+
let node_idx = self.dep_graph.add_node(node_kind);
134+
self.locals_to_node_indexes.insert(local, node_idx);
135+
node_idx
136+
}
137+
}
125138
}
126139

127140
impl<'a, 'tcx> Visitor<'tcx> for BorrowDependencies<'a, 'tcx> {
@@ -130,7 +143,7 @@ impl<'a, 'tcx> Visitor<'tcx> for BorrowDependencies<'a, 'tcx> {
130143
// Add nodes for the arguments
131144
for i in 1..=body.arg_count {
132145
let local = Local::from_usize(i);
133-
let node_kind = NodeKind::LocalOrLocalWithRefs(local);
146+
let node_kind = NodeKind::Local(local);
134147
let node_idx = self.dep_graph.add_node(node_kind);
135148
self.locals_to_node_indexes.insert(local, node_idx);
136149
}
@@ -144,20 +157,8 @@ impl<'a, 'tcx> Visitor<'tcx> for BorrowDependencies<'a, 'tcx> {
144157
StatementKind::Assign(assign) => {
145158
let assign_place = assign.0;
146159
let assign_local = assign_place.local;
147-
148-
let node_kind = match assign_place.ty(self.local_decls, self.tcx).ty.kind() {
149-
ty::Ref(..) | ty::RawPtr(..) => NodeKind::Borrow(assign_local),
150-
_ => NodeKind::LocalOrLocalWithRefs(assign_local),
151-
};
152-
153-
let node_idx = self.dep_graph.add_node(node_kind);
154-
self.locals_to_node_indexes.insert(assign_local, node_idx);
155-
self.current_local = Some(node_idx);
156-
157-
debug!(
158-
"set current_local to {:?} for local {:?}",
159-
self.current_local, assign_local
160-
);
160+
self.current_local = Some(assign_local);
161+
debug!("set current_local to {:?}", self.current_local);
161162

162163
// Do not call `visit_place` here as this might introduce a self-edge, which our liveness analysis
163164
// assumes not to exist.
@@ -178,18 +179,53 @@ impl<'a, 'tcx> Visitor<'tcx> for BorrowDependencies<'a, 'tcx> {
178179
fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, location: Location) {
179180
debug!("self.current_local: {:?}", self.current_local);
180181
match rvalue {
182+
Rvalue::Use(Operand::Move(place) | Operand::Copy(place))
183+
if matches!(
184+
place.ty(self.local_decls, self.tcx).ty.kind(),
185+
ty::Ref(..) | ty::RawPtr(..)
186+
) =>
187+
{
188+
// these are just re-assignments of already outstanding refs or pointers,
189+
// hence we want to treat them as `NodeKind::Borrow`
190+
// FIXME Are these always Operand::Copy or is Operand::Move also possible for refs/ptrs?
191+
let Some(src_local) = self.current_local else {
192+
bug!("Expected self.current_local to be set with Rvalue::Ref|Rvalue::AddressOf");
193+
};
194+
195+
// These are just moves of refs/ptrs, hence `NodeKind::Borrow`.
196+
let src_node_kind = NodeKind::Borrow(src_local);
197+
let src_node_idx = self.maybe_create_node(src_node_kind);
198+
199+
let node_idx = if let Some(node_idx) = self.locals_to_node_indexes.get(&place.local)
200+
{
201+
*node_idx
202+
} else {
203+
bug!("should have added a node for the moved borrowed place before");
204+
};
205+
206+
debug!(
207+
"adding edge from {:?}({:?}) -> {:?}({:?})",
208+
src_node_idx,
209+
self.dep_graph.node(src_node_idx).data,
210+
node_idx,
211+
self.dep_graph.node(node_idx).data,
212+
);
213+
214+
self.dep_graph.add_edge(src_node_idx, node_idx, ());
215+
}
181216
Rvalue::Ref(_, _, borrowed_place) | Rvalue::AddressOf(_, borrowed_place) => {
182-
let Some(src_node_idx) = self.current_local else {
217+
let Some(src_local) = self.current_local else {
183218
bug!("Expected self.current_local to be set with Rvalue::Ref|Rvalue::AddressOf");
184219
};
185220

186-
let borrowed_local = borrowed_place.local;
187-
let node_idx =
188-
if let Some(node_idx) = self.locals_to_node_indexes.get(&borrowed_local) {
189-
*node_idx
190-
} else {
191-
self.dep_graph.add_node(NodeKind::Borrow(borrowed_place.local))
192-
};
221+
// we're in a statement like `_4 = Ref(..)`, hence NodeKind::Borrow for `_4`
222+
let src_node_kind = NodeKind::Borrow(src_local);
223+
let src_node_idx = self.maybe_create_node(src_node_kind);
224+
225+
// If we haven't previously added a node for `borrowed_place.local` then it can be neither
226+
// `NodeKind::Borrow` nor `NodeKind::LocalsWithRefs`.
227+
let borrowed_node_kind = NodeKind::Local(borrowed_place.local);
228+
let node_idx = self.maybe_create_node(borrowed_node_kind);
193229

194230
debug!(
195231
"adding edge from {:?}({:?}) -> {:?}({:?})",
@@ -213,14 +249,16 @@ impl<'a, 'tcx> Visitor<'tcx> for BorrowDependencies<'a, 'tcx> {
213249
debug!("current local: {:?}", self.current_local);
214250
match place_ty.kind() {
215251
ty::Ref(..) | ty::RawPtr(..) => match self.current_local {
216-
Some(src_node_idx) => {
217-
let borrowed_local = place.local;
218-
let node_idx =
219-
if let Some(node_idx) = self.locals_to_node_indexes.get(&borrowed_local) {
220-
*node_idx
221-
} else {
222-
self.dep_graph.add_node(NodeKind::Borrow(borrowed_local))
223-
};
252+
Some(src_local) => {
253+
// If we haven't created a node for this before, then this must be a
254+
// `NodeKind::LocalWithRefs` as we would have handled the
255+
// other possible assignment case (`NodeKind::Local`) previously in
256+
// `visit_rvalue`.
257+
let src_node_kind = NodeKind::LocalWithRefs(src_local);
258+
let src_node_idx = self.maybe_create_node(src_node_kind);
259+
260+
let borrowed_node_kind = NodeKind::Borrow(place.local);
261+
let node_idx = self.maybe_create_node(borrowed_node_kind);
224262

225263
debug!(
226264
"adding edge from {:?}({:?}) -> {:?}({:?})",
@@ -268,13 +306,8 @@ impl<'a, 'tcx> Visitor<'tcx> for BorrowDependencies<'a, 'tcx> {
268306
false
269307
};
270308

271-
let node_idx =
272-
self.dep_graph.add_node(NodeKind::LocalOrLocalWithRefs(destination.local));
273-
debug!("added local {:?} to graph with idx {:?}", destination.local, node_idx);
274-
self.locals_to_node_indexes.insert(destination.local, node_idx);
275-
276309
if dest_might_include_refs_or_pointers {
277-
self.current_local = Some(node_idx);
310+
self.current_local = Some(destination.local);
278311
debug!("self.current_local: {:?}", self.current_local);
279312
}
280313

@@ -286,10 +319,29 @@ impl<'a, 'tcx> Visitor<'tcx> for BorrowDependencies<'a, 'tcx> {
286319
self.current_local = None;
287320
}
288321
TerminatorKind::Yield { resume_arg, value, .. } => {
289-
let node_idx =
290-
self.dep_graph.add_node(NodeKind::LocalOrLocalWithRefs(resume_arg.local));
291-
debug!("added local {:?} to graph with idx {:?}", resume_arg.local, node_idx);
292-
self.locals_to_node_indexes.insert(resume_arg.local, node_idx);
322+
let resume_arg_ty = resume_arg.ty(self.local_decls, self.tcx).ty;
323+
debug!(?resume_arg_ty);
324+
325+
let mut has_refs_or_pointers_visitor = MaybeHasRefsOrPointersVisitor::new();
326+
resume_arg_ty.visit_with(&mut has_refs_or_pointers_visitor);
327+
328+
let dest_might_include_refs_or_pointers =
329+
if has_refs_or_pointers_visitor.has_refs_or_pointers {
330+
true
331+
} else if has_refs_or_pointers_visitor.has_params {
332+
// if we pass in any references or raw pointers as arguments and the
333+
// return type includes type parameters, these type parameters could
334+
// refer to those refs/ptrs, so we have to be conservative here and possibly
335+
// include an edge.
336+
true
337+
} else {
338+
false
339+
};
340+
341+
if dest_might_include_refs_or_pointers {
342+
self.current_local = Some(resume_arg.local);
343+
debug!("self.current_local: {:?}", self.current_local);
344+
}
293345

294346
self.super_operand(value, location);
295347
}
@@ -397,7 +449,7 @@ where
397449
// base node to keep alive
398450
vec![src_node.data.get_local()]
399451
} else {
400-
if matches!(src_node.data, NodeKind::LocalOrLocalWithRefs(_)) {
452+
if matches!(src_node.data, NodeKind::LocalWithRefs(_)) {
401453
// These are locals that we need to keep alive, but that also contain
402454
// successors in the graph since they contain other references/pointers.
403455
locals_for_node.push(current_local);
@@ -577,7 +629,7 @@ where
577629
match projection.as_slice() {
578630
&[] | &[ProjectionElem::OpaqueCast(_)] => {
579631
// If there aren't any projections or just an OpaqueCast we need to
580-
// kill the local if it's a ref or a pointer.
632+
// kill the local.
581633
match lhs_place_ty.kind() {
582634
ty::Ref(..) | ty::RawPtr(..) => {
583635
debug!("killing {:?}", lhs_place.local);
@@ -591,7 +643,7 @@ where
591643
}
592644
}
593645
_ => {
594-
// With any other projection elements a projection of a local (of type ref/ptr)
646+
// With any other projection elements, a projection of a local (of type ref/ptr)
595647
// is actually a use-site, but we handle this in the call to `visit_place`.
596648
self.super_assign(&assign.0, &assign.1, location);
597649
}

0 commit comments

Comments
 (0)