Skip to content
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

mlir: Add Enzyme ops removal on structured control flow #2200

Merged
merged 10 commits into from
Jan 5, 2025

Conversation

Pangoraw
Copy link
Contributor

@Pangoraw Pangoraw commented Dec 18, 2024

TODO:

  • scf: support non-constant iterations Cache<f32> -> tensor<?xf32>.
  • scf: push/pop only once if a value is pushed multiple times.
  • Cache of tensor (nested for).
  • passes: add option in enzyme pass to try to remove enzyme ops after generating the function. This should help with higher order diff. ref MLIR: post optimization pipeline #2214.
  • scf: graph min-cut.

return mlir::enzyme::CacheInfo::batchType(mlir::ShapedType::kDynamic);
}

mlir::Type mlir::enzyme::CacheInfo::batchType(int64_t dim) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so there is already an Enzyme Autodiff Type interface, which should have a method for batching (and if not that would probably be the right place for this)

@Pangoraw Pangoraw marked this pull request as ready for review January 3, 2025 14:44
assert(width == 1 && "unsupported width != 1");
return self;
Type getShadowType(Type self, int64_t width) const {
return batchType(self, width);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in a separate different PR, it may be worthwhile switching getShadowType and the likes to take an ArrayRef<int64_t> indices to batch on (@jumerckx did something similar when adding batched differentiation broadcast earlier)

Copy link
Member

@wsmoses wsmoses left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, though there's some unrelated batch stuff here that probably shouldn't be here (maybe leftover from debugging)

@Pangoraw
Copy link
Contributor Author

Pangoraw commented Jan 5, 2025

though there's some unrelated batch stuff here that probably shouldn't be here (maybe leftover from debugging)

Yeah I had cherry-picked changes from #2181 before it was merged, the diff should be more correct now that I rebased.

@wsmoses wsmoses merged commit 5b330a9 into EnzymeAD:main Jan 5, 2025
12 of 21 checks passed
@Pangoraw Pangoraw deleted the remove-ops branch January 5, 2025 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants