-
Notifications
You must be signed in to change notification settings - Fork 15
feat: update passes to use PassScope where non-breaking #2836
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
Changes from 52 commits
1f05f19
aba6477
3e0c6ac
02d2886
e3a63b4
0b730bf
e71125c
11a643e
62bd044
ce4c551
293ef0f
075e2b6
236c08c
b16b72a
84b33c7
d7eca10
51bfb59
3e6997c
b9cac77
f22c27e
81f2cac
c1a1898
90172c5
d678210
d6eba57
3295c8e
f98093d
14c5b9b
a3e07c9
540dab7
a6ebb96
85a094d
834edd9
22e951a
6a4332d
968c12e
332bc84
1c38f50
6b05449
db7d78f
756b310
470a970
134784a
b9e6bfb
7180479
75a2d14
3c152af
17e878c
89a5a17
3dd5187
1866af0
4bb84f8
7a03521
1e2b9d5
e3c9eee
c8c6e76
82cfd0e
79641a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,14 +6,16 @@ use std::collections::{HashMap, HashSet, VecDeque}; | |
| use std::fmt::{Debug, Display, Formatter}; | ||
| use std::sync::Arc; | ||
|
|
||
| use crate::ComposablePass; | ||
| use crate::{ComposablePass, PassScope}; | ||
|
|
||
| /// Configuration for Dead Code Elimination pass | ||
| #[derive(Clone)] | ||
| pub struct DeadCodeElimPass<H: HugrView> { | ||
| /// Nodes that are definitely needed - e.g. `FuncDefns`, but could be anything. | ||
| /// Hugr Root is assumed to be an entry point even if not mentioned here. | ||
| entry_points: Vec<H::Node>, | ||
| /// If None, use entrypoint-subtree (even if module root) | ||
| scope: Option<PassScope>, | ||
| /// Callback identifying nodes that must be preserved even if their | ||
| /// results are not used. Defaults to [`PreserveNode::default_for`]. | ||
| preserve_callback: Arc<PreserveCallback<H>>, | ||
|
|
@@ -23,6 +25,8 @@ impl<H: HugrView + 'static> Default for DeadCodeElimPass<H> { | |
| fn default() -> Self { | ||
| Self { | ||
| entry_points: Default::default(), | ||
| // Preserve pre-PassScope behaviour of affecting entrypoint subtree only: | ||
| scope: None, | ||
| preserve_callback: Arc::new(PreserveNode::default_for), | ||
| } | ||
| } | ||
|
|
@@ -36,11 +40,13 @@ impl<H: HugrView> Debug for DeadCodeElimPass<H> { | |
| #[derive(Debug)] | ||
| struct DCEDebug<'a, N> { | ||
| entry_points: &'a Vec<N>, | ||
| scope: &'a Option<PassScope>, | ||
| } | ||
|
|
||
| Debug::fmt( | ||
| &DCEDebug { | ||
| entry_points: &self.entry_points, | ||
| scope: &self.scope, | ||
| }, | ||
| f, | ||
| ) | ||
|
|
@@ -97,11 +103,11 @@ impl<H: HugrView> DeadCodeElimPass<H> { | |
| self | ||
| } | ||
|
|
||
| /// Mark some nodes as entry points to the Hugr, i.e. so we cannot eliminate any code | ||
| /// used to evaluate these nodes. | ||
| /// [`HugrView::entrypoint`] is assumed to be an entry point; | ||
| /// for Module roots the client will want to mark some of the `FuncDefn` children | ||
| /// as entry points too. | ||
| /// Mark some nodes as starting points for analysis, i.e. so we cannot eliminate any code | ||
| /// used to evaluate these nodes. (E.g. nodes at which we may start executing the Hugr.) | ||
| /// | ||
| /// Other starting points are added according to the [PassScope]. | ||
| // TODO should we deprecate this? i.e. require use of PreserveCallback / Hugr edges? | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. However, deprecating this would make it impossible to support
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would crate-private be enough for testing?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah possibly. But maybe the answer to that is to move it into tket rather than being its own crate?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Soon™️ (We'll first move the |
||
| pub fn with_entry_points(mut self, entry_points: impl IntoIterator<Item = H::Node>) -> Self { | ||
| self.entry_points.extend(entry_points); | ||
| self | ||
|
|
@@ -111,14 +117,21 @@ impl<H: HugrView> DeadCodeElimPass<H> { | |
| let mut must_preserve = HashMap::new(); | ||
| let mut needed = HashSet::new(); | ||
| let mut q = VecDeque::from_iter(self.entry_points.iter().copied()); | ||
| q.push_front(h.entrypoint()); | ||
|
|
||
| match &self.scope { | ||
| None => q.push_back(h.entrypoint()), | ||
| Some(scope) => q.extend(scope.preserve_interface(h)), | ||
| }; | ||
| while let Some(n) = q.pop_front() { | ||
| if !h.contains_node(n) { | ||
| return Err(DeadCodeElimError::NodeNotFound(n)); | ||
| } | ||
| if !needed.insert(n) { | ||
| continue; | ||
| } | ||
| // Ensure no orphans. We could remove more from parent, but would require transforming | ||
| // (e.g. removing individual Output ports) not just deleting, so don't. | ||
| q.extend(h.get_parent(n)); | ||
acl-cqc marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| for (i, ch) in h.children(n).enumerate() { | ||
| if self.must_preserve(h, &mut must_preserve, ch) | ||
| || match h.get_optype(ch) { | ||
|
|
@@ -181,16 +194,28 @@ impl<H: HugrMut> ComposablePass<H> for DeadCodeElimPass<H> { | |
| type Result = (); | ||
|
|
||
| fn run(&self, hugr: &mut H) -> Result<(), Self::Error> { | ||
| let root = match &self.scope { | ||
| None => hugr.entrypoint(), | ||
| Some(scope) => match scope.root(hugr) { | ||
| Some(root) => root, | ||
| None => return Ok(()), | ||
| }, | ||
| }; | ||
| let needed = self.find_needed_nodes(&*hugr)?; | ||
| let remove = hugr | ||
| .entry_descendants() | ||
| .descendants(root) | ||
| .filter(|n| !needed.contains(n)) | ||
| .collect::<Vec<_>>(); | ||
| for n in remove { | ||
| hugr.remove_node(n); | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
| fn with_scope_internal(mut self, scope: impl Into<PassScope>) -> Self { | ||
| self.scope = Some(scope.into()); | ||
| self | ||
| } | ||
| } | ||
| #[cfg(test)] | ||
| mod test { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, this could be outside test, even public; it's a simple-enough utility function