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.

Sign up for GitHub

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

Jump to bottom

Added reverse mode adjoint for linalg.generic #1134

Merged
merged 19 commits into from
Jul 11, 2023
Merged

Added reverse mode adjoint for linalg.generic #1134

merged 19 commits into from
Jul 11, 2023

Conversation

umatin
Copy link
Collaborator

@umatin umatin commented Apr 24, 2023

This has been tested on the MLIR-Reverse fork without activity analysis. With activity analysis I am not able to compile differentiate any code containing memrefs. This would probably have to be fixed before this can be merged.

tgymnich
tgymnich reviewed Apr 24, 2023
View reviewed changes
enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Implementations/SCFAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Interfaces/EnzymeLogicReverse.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Interfaces/GradientUtilsReverse.cpp Show resolved Hide resolved
enzyme/Enzyme/MLIR/Interfaces/GradientUtilsReverse.cpp Show resolved Hide resolved
enzyme/Enzyme/MLIR/Interfaces/GradientUtilsReverse.h Show resolved Hide resolved
@umatin umatin marked this pull request as ready for review May 12, 2023 11:17
ftynse
ftynse reviewed May 16, 2023
View reviewed changes
enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Passes/SimplifyMemrefCache.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Passes/Passes.td Outdated Show resolved Hide resolved
@umatin umatin requested a review from ftynse June 2, 2023 12:38
ftynse
ftynse reviewed Jun 2, 2023
View reviewed changes
Copy link
Collaborator

@ftynse ftynse left a comment

Choose a reason for hiding this comment

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

There is one place where I suspect the adjoint generator produces invalid IR. Otherwise looks good. Some tests would be beneficial.

enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Show resolved Hide resolved
enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Implementations/LinalgAutoDiffOpInterfaceImpl.cpp Outdated Show resolved Hide resolved
enzyme/Enzyme/MLIR/Interfaces/EnzymeLogicReverse.cpp Outdated Show resolved Hide resolved
ftynse
ftynse reviewed Jun 2, 2023
View reviewed changes
Copy link
Collaborator

@ftynse ftynse left a comment

Choose a reason for hiding this comment

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

There is one place where I suspect the adjoint generator produces invalid IR. Otherwise looks good. Some tests would be beneficial.

@pengmai pengmai mentioned this pull request Jul 7, 2023
Closed
@ftynse ftynse merged commit 3d35e80 into main Jul 11, 2023
53 of 64 checks passed
@ftynse ftynse deleted the linalg.generic branch July 11, 2023 13:07
@Porkepix Porkepix mentioned this pull request Jul 13, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ftynse ftynse ftynse left review comments

@wsmoses wsmoses wsmoses approved these changes

@tgymnich tgymnich tgymnich approved these changes

@pengmai pengmai Awaiting requested review from pengmai

Assignees

@umatin umatin

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@umatin @wsmoses @ftynse @tgymnich @pengmai