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

Allow quantum registers returned from functions during buffer deallocation #1408

Merged
merged 10 commits into from
Jan 8, 2025

Conversation

dime10
Copy link
Contributor

@dime10 dime10 commented Jan 3, 2025

Context: This patch fixes a bug in upstream MLIR that prevents buffer deallocation from working in certain cases. The patch was also submitted upstream: llvm/llvm-project#121582

Description of the Change: This PR adds a patch to the llvm-project source, and updates the relevant build recipes. The llvm step during wheel builds is now also handled through Make instead of replicating the CMake configuration in each wheel script.

Benefits: Unblocks our friends over at Qrisp.

Possible Drawbacks: Patching dependencies is not great, but hopefully temporary.

Related GitHub Issues:
[sc-81444]

@dime10 dime10 added bug Something isn't working upstream Issue with an upstream project labels Jan 3, 2025
@dime10 dime10 requested a review from erick-xanadu January 3, 2025 23:23
Copy link
Contributor

github-actions bot commented Jan 3, 2025

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.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.

@dime10 dime10 added the author:build-wheels Run the wheel building workflows on this Pull Request label Jan 3, 2025
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.

Thanks for using the make recipes. In the future we can add the patches to the cache's hash. It looks like the build process failed, I'll give you some more time to work on it. Let me know again once it is succeeding.

mlir/Makefile Show resolved Hide resolved
mlir/Makefile Show resolved Hide resolved
@dime10 dime10 force-pushed the buffer-deallocation-patch branch from 2a772be to f9f4e84 Compare January 6, 2025 23:15
@dime10 dime10 added this to the v0.10.0 milestone Jan 7, 2025
It's the only wheel build script that uses it.
@dime10
Copy link
Contributor Author

dime10 commented Jan 7, 2025

Thanks for using the make recipes. In the future we can add the patches to the cache's hash. It looks like the build process failed, I'll give you some more time to work on it. Let me know again once it is succeeding.

At last the builds are passing 😰 On the other hand I did some cleaning in the process ^^

@dime10 dime10 removed the author:build-wheels Run the wheel building workflows on this Pull Request label Jan 7, 2025
@dime10 dime10 requested a review from erick-xanadu January 8, 2025 16:43
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.

🥳

@dime10 dime10 merged commit b8f6cb8 into main Jan 8, 2025
60 checks passed
@dime10 dime10 deleted the buffer-deallocation-patch branch January 8, 2025 16:52
dime10 added a commit that referenced this pull request Jan 8, 2025
…ation (#1408)

**Context:** This patch fixes a bug in upstream MLIR that prevents
buffer deallocation from working in certain cases. The patch was also
submitted upstream: llvm/llvm-project#121582

**Description of the Change:** This PR adds a patch to the llvm-project
source, and updates the relevant build recipes. The llvm step during
wheel builds is now also handled through Make instead of replicating the
CMake configuration in each wheel script.

**Benefits:** Unblocks our friends over at Qrisp.

**Possible Drawbacks:** Patching dependencies is not great, but
hopefully temporary.

**Related GitHub Issues:**
[sc-81444]
dime10 added a commit that referenced this pull request Jan 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working upstream Issue with an upstream project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants