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

Update Jax(LLVM, MLIR) and Enzyme #428

Merged
merged 38 commits into from
Feb 22, 2024
Merged

Update Jax(LLVM, MLIR) and Enzyme #428

merged 38 commits into from
Feb 22, 2024

Conversation

rmoyard
Copy link
Contributor

@rmoyard rmoyard commented Jan 4, 2024

Description of the Change:
NullOp -> ZeroOp
llvm/llvm-project#67183

CodeGenOpt -> CodeGenOptLevel
https://github.com/llvm/llvm-project/pull/66295/files

Converters are const
llvm/llvm-project@ce25459

Remove typed pointer
llvm/llvm-project#71285

thlo gml_st on ice
tensorflow/mlir-hlo@7077cec

ByteCode is a new interface
llvm/llvm-project@837d1ce

getArgsOperandMutable was added CallOpInterface
llvm/llvm-project@d790a21

Remove redundant memref.tensor_store op
https://github.com/llvm/llvm-project/pull/71010/files

Lowering of memref.realloc was extracted to its own pass
https://reviews.llvm.org/D159430

Benefits:

Possible Drawbacks:

Related GitHub Issues:

Copy link
Contributor

github-actions bot commented Jan 5, 2024

Hello. You may have forgotten to update the changelog!
Please edit doc/changelog.md on your branch with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@rmoyard
Copy link
Contributor Author

rmoyard commented Jan 5, 2024

[sc-53151]

@rmoyard
Copy link
Contributor Author

rmoyard commented Jan 16, 2024

Blocked by EnzymeAD/Enzyme#1608

@rmoyard rmoyard marked this pull request as ready for review February 21, 2024 18:26
@dime10 dime10 added this to the v0.5.0 milestone Feb 21, 2024
Copy link

codecov bot commented Feb 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (96fef3b) 99.52% compared to head (c550989) 99.52%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #428   +/-   ##
=======================================
  Coverage   99.52%   99.52%           
=======================================
  Files          45       45           
  Lines        7941     7942    +1     
  Branches      537      537           
=======================================
+ Hits         7903     7904    +1     
  Misses         20       20           
  Partials       18       18           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rmoyard rmoyard added the author:build-wheels Run the wheel building workflows on this Pull Request label Feb 21, 2024
@dime10 dime10 added the reviewer:require-wheels Pull Requests will need wheel building job successful before being merged label Feb 21, 2024
Copy link
Contributor

@erick-xanadu erick-xanadu 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

rmoyard and others added 2 commits February 21, 2024 17:09
Co-authored-by: erick-xanadu <110487834+erick-xanadu@users.noreply.github.com>
Copy link
Collaborator

@dime10 dime10 left a comment

Choose a reason for hiding this comment

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

Thanks @rmoyard for this long awaited update 💯

.dep-versions Show resolved Hide resolved
@@ -128,7 +128,6 @@ def run_writing_command(command: List[str], compile_options: Optional[CompileOpt
"func.func(hlo-legalize-to-linalg)",
"func.func(mhlo-legalize-to-std)",
"convert-to-signless",
"func.func(scalarize)",
Copy link
Collaborator

Choose a reason for hiding this comment

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

@erick-xanadu I remember you found that either of two passes was providing similar speedups, one at the hlo level and one at the linalg level. Can we replace this pass with the linalg one (since it no longer exists)?

@@ -179,6 +178,7 @@ def run_writing_command(command: List[str], compile_options: Optional[CompileOpt
MLIR_TO_LLVM_PASS = (
"MLIRToLLVMDialect",
[
"expand-realloc",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Small comment why this pass is necessary will be nice, since it's hard to see why pipelines are the way they are.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will get the reference I don't remember where it comes from

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://reviews.llvm.org/D159430 This was part of the memref lowering and it was extracted to make lowering simpler

Copy link
Collaborator

Choose a reason for hiding this comment

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

Right I meant adding it as a comment into the file

frontend/test/lit/test_aot_compilation.py Show resolved Hide resolved
mlir/test/Catalyst/BufferizationTest.mlir Show resolved Hide resolved
mlir/test/Quantum/WrapperTest.mlir Outdated Show resolved Hide resolved
rmoyard and others added 2 commits February 21, 2024 17:25
Co-authored-by: David Ittah <dime10@users.noreply.github.com>
@rmoyard rmoyard merged commit 8649c9e into main Feb 22, 2024
55 of 56 checks passed
@rmoyard rmoyard deleted the update_jax_llvm_mlir_enzyme branch February 22, 2024 01:40
rauletorresc pushed a commit that referenced this pull request Feb 26, 2024
**Description of the Change:**
NullOp -> ZeroOp
llvm/llvm-project#67183

CodeGenOpt -> CodeGenOptLevel
https://github.com/llvm/llvm-project/pull/66295/files

Converters are const

llvm/llvm-project@ce25459

Remove typed pointer
llvm/llvm-project#71285

thlo gml_st on ice

tensorflow/mlir-hlo@7077cec

ByteCode is a new interface

llvm/llvm-project@837d1ce

getArgsOperandMutable was added CallOpInterface

llvm/llvm-project@d790a21

Remove redundant memref.tensor_store op
https://github.com/llvm/llvm-project/pull/71010/files

Lowering of memref.realloc was extracted to its own pass
https://reviews.llvm.org/D159430

**Benefits:**

**Possible Drawbacks:**

**Related GitHub Issues:**

---------

Co-authored-by: erick-xanadu <110487834+erick-xanadu@users.noreply.github.com>
Co-authored-by: David Ittah <dime10@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author:build-wheels Run the wheel building workflows on this Pull Request reviewer:require-wheels Pull Requests will need wheel building job successful before being merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants