-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat!: don't assume "main"-function in dataflow + constant folding #1896
base: main
Are you sure you want to change the base?
Conversation
This PR contains breaking changes to the public Rust API. cargo-semver-checks summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## acl/const_fold_dce #1896 +/- ##
======================================================
- Coverage 86.54% 86.52% -0.02%
======================================================
Files 195 195
Lines 35652 35594 -58
Branches 32465 32407 -58
======================================================
- Hits 30854 30799 -55
+ Misses 3011 3009 -2
+ Partials 1787 1786 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
let mut p = in_values.into_iter().peekable(); | ||
// We must provide some inputs to the root so that they are Top rather than Bottom. | ||
// (However, this test will fail for DataflowBlock or Conditional roots, i.e. if no | ||
// inputs have been provided they will still see Bottom. We could store the "input" |
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.
This is a very long note to have as a "We could" comment, it'll probably just get out-of-date ;). Perhaps I should just do it now...
Less efficient - we add constants for all ops to the Hugr, many of which we then DCE. And, fails a test, because DCE doesn't have loop-termination results
733de9c
to
9f477b0
Compare
Will make this a follow-on to #1901 |
Instead, move towards explicitly stating entry-points, in common with
remove_dead_funcs
andremove_dead_code
in #1897 - note this PR follows #1897.I've also tried to reorganise the code a bit better wrt.
prepopulate_df_inputs
as we can also handleConditional
andCFG
nodes by using existing code, hence renaming to justprepopulate_inputs
BREAKING CHANGE: dataflow analysis on Module-rooted Hugrs will no longer accept input PartialValues for the main function, these should be passed to
prepopulate_inputs
instead. Alsoprepopulate_df_inputs
is gone, again useprepopulate_inputs
. ConstantFoldPass::with_inputs now takes the entry-point node as first parameter as well as the inputs to apply there.