-
Notifications
You must be signed in to change notification settings - Fork 0
Ad ptr merge #1
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
base: autodiff
Are you sure you want to change the base?
Ad ptr merge #1
Conversation
@@ -29,6 +49,7 @@ add_thorin_dialect(clos | |||
clos/phase/lower_typed_clos.cpp | |||
clos/phase/lower_typed_clos.h | |||
mem/passes/fp/copy_prop.cpp | |||
mem/passes/rw/reshape.cpp |
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.
Check if inter-dialect phases are necessary here.
|
||
auto cmp = core::op(core::icmp::ul, iter, end); | ||
for_lam->branch(false, cmp, new_body, if_else, w.tuple()); | ||
for_lam->branch(false, cmp, new_body, if_else, acc); |
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.
@christopherhjung
Is the inlining/forwarding of the accumulator necessary here?
Optimizations like CopyProp will probably delete the arguments.
And Closure Conversion has as one main function closing open terms and will (if not inlined) re-add the accumulator and bound variables as arguments.
@christopherhjung Reshape has problems with empty arguments (or higher-order arguments). |
Pin the MacOS runner to MacOS 11.x, since the 12.x makes the download-release-asset action fail. Since I don't know much about Node.JS I opened an issue against the action's repo: i3h/download-release-asset#9
[CI] Downgrade MacOS Runner to 11 for now
…s remain for now)
To review the changes and keep track of changes.