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

[CircuitCheck] Run canonicalizer before checking #122

Merged
merged 1 commit into from
May 3, 2023

Conversation

boschmitt
Copy link
Collaborator

Description

CircuitCheck might failed if we don't canonicalize the IR before trying to build the unitary. This step is mainly to guarantee that operators parameters that are known at compilation time are defined by ArithDialect's constant operation. For example:

// CircuitCheck _cannot_ handle:
%cst = arith.constant 3.1415926535897931 : f64
%neg_cst = arith.negf %cst : f64
quake.rx |%neg_cst : f64| (%q0)

// CircuitCheck can handle:
%cst = arith.constant -3.1415926535897931 : f64
quake.rx |%cst : f64| (%q0)

This PR also:

  • Improves error reporting
  • Fixes the computation of global phase

Copy link
Collaborator

@amccaskey amccaskey left a comment

Choose a reason for hiding this comment

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

Do you think there's a test that you could add showing the canonicalization working here for CircuitCheck? Overall LGTM

@boschmitt
Copy link
Collaborator Author

Do you think there's a test that you could add showing the canonicalization working here for CircuitCheck? Overall LGTM

I will open an issue to come up with CircuitCheck specific tests, now the tool is pretty much hand-tested. Canonicalization itself is tested in MLIR. The use of canonicalization in this tool will be put to test when checking if decompositions patters are correctly transforming the IR.

@boschmitt boschmitt enabled auto-merge (squash) May 3, 2023 10:06
@boschmitt boschmitt merged commit 48f9f23 into NVIDIA:main May 3, 2023
@github-actions github-actions bot locked and limited conversation to collaborators May 3, 2023
@bettinaheim bettinaheim added the release notes Changes need to be captured in the release notes label Jun 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
release notes Changes need to be captured in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants