Skip to content

⏪️ [MLIR] Bring back MLIR / LLVM 19.0 support #934

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

Merged
merged 7 commits into from
Apr 24, 2025
Merged

Conversation

flowerthrower
Copy link
Collaborator

@flowerthrower flowerthrower commented Apr 23, 2025

Description

Reincarnate MLIR / LLVM 19.0 support.
Necessary for Catalyst-related efforts (like #880).

Checklist:

  • The pull request only contains commits that are related to it.
  • I have added appropriate tests and documentation.
  • I have made sure that all CI jobs on GitHub pass.
  • The pull request introduces no new warnings and follows the project's style guidelines.

@flowerthrower flowerthrower added fix Fix for something that isn't working MLIR Anything related to MLIR labels Apr 23, 2025
Copy link

codecov bot commented Apr 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

Copy link
Member

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Just some minor fixes necessary.

@burgholzer burgholzer changed the title [MLIR] Bring back MLIR / LLVM 19.0 support ⏪️ [MLIR] Bring back MLIR / LLVM 19.0 support Apr 23, 2025
@burgholzer
Copy link
Member

Probably also worth a changelog entry.

@flowerthrower
Copy link
Collaborator Author

Thanks @burgholzer for the quick review. I added the requested changes.
We currently only test with v20, which is an issue, as the behaviour of the transformation passes is (somehow) different in v19 and the CHECKs inside of Dialect/MQTOpt/Transforms/branch-opt.mlir and Dialect/MQTOpt/Transforms/branch-opt. mlir fail. @ystade was so kind to offer support in this regard.

@burgholzer
Copy link
Member

Thanks @burgholzer for the quick review. I added the requested changes. We currently only test with v20, which is an issue, as the behaviour of the transformation passes is (somehow) different in v19 and the CHECKs inside of Dialect/MQTOpt/Transforms/branch-opt.mlir and Dialect/MQTOpt/Transforms/branch-opt. mlir fail. @ystade was so kind to offer support in this regard.

Sounds good to me! The current code changes all look reasonable.
What is still missing is re-enabling the CI on LLVM@19.
That should also surface the error you are talking about.
You should be able to simply revert the ci_mlir.yml file to a previous version that still tested for both versions.

…ng MLIR subdirectory (#895)"

This partially reverts commit c844c00.
@ystade
Copy link
Collaborator

ystade commented Apr 24, 2025

We know that applyPatternsGreedilyAndFold(...) still exists in LLVM 20 but is deprecated. Assuming that this function has the identical behaviour in LLVM 19 and LLVM 20, in our use case, it should even be identical to the function applyPatternsGreedily(...) newly introduced in LLVM 20. The deprecated function applyPatternsGreedilyAndFold(...) in LLVM just sets the field config.fold = true and then calls applyPatternsGreedily(...). Since we are not specifying any particular config and the default value for the fold member is true, both function should have the same behaviour in the end.

@ystade
Copy link
Collaborator

ystade commented Apr 24, 2025

Apparently, the latest minor version of LLVM 19 does not cause any issues in our tests, and this PR should be ready to go in. Is that correct, @burgholzer?

@flowerthrower
Copy link
Collaborator Author

Thank you @ystade! Looks like this is only an issue with the v19 that is used (and possibly patched) in Catalyst---the one I have been using on my local machine.

@burgholzer burgholzer marked this pull request as ready for review April 24, 2025 13:07
@burgholzer burgholzer moved this to In Progress in MQT Core Apr 24, 2025
Copy link
Member

@burgholzer burgholzer left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼

@burgholzer burgholzer enabled auto-merge (squash) April 24, 2025 13:08
@burgholzer burgholzer merged commit 987a592 into main Apr 24, 2025
31 checks passed
@burgholzer burgholzer deleted the mlir-v19-revival branch April 24, 2025 13:10
@github-project-automation github-project-automation bot moved this from In Progress to Done in MQT Core Apr 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Fix for something that isn't working MLIR Anything related to MLIR
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants