-
Notifications
You must be signed in to change notification settings - Fork 13
fix: Fix inline_constant_functions pass
#2135
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2135 +/- ##
=======================================
Coverage 82.15% 82.16%
=======================================
Files 227 227
Lines 39891 39897 +6
Branches 35990 35996 +6
=======================================
+ Hits 32774 32781 +7
+ Misses 5287 5286 -1
Partials 1830 1830
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
doug-q
left a comment
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.
Thanks Mark, one suggestion
| hugr.insert_hugr(func_node, func_hugr); | ||
| func_hugr.replace_op(func_hugr.root(), func_defn).unwrap(); | ||
| let func_node = hugr.insert_hugr(hugr.root(), func_hugr).new_root; | ||
| hugr.set_num_ports(func_node, 0, 1); |
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.
use add_ports here, and use the port it returns
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 about removing ports: If the inlined function value is a DFG, then it already has some inputs/outputs that we need to remove because it's now a FuncDefn
| hugr.replace_op(lcn, LoadFunction::try_new(polysignature.clone(), [])?)?; | ||
|
|
||
| let src_port = hugr.node_outputs(func_node).next().unwrap(); | ||
| let tgt_port = hugr.node_inputs(lcn).next().unwrap(); |
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.
It would be better to delegate to LoadConstant what the input is. You don't have one at hand, so maybe not worth it.
15988dd to
527ad1c
Compare
Fixes #2132