KEMBAR78
Fix constant folding of booleans casted to integers by annagrin · Pull Request #2805 · NVIDIA/cuda-quantum · GitHub
Skip to content

Fix constant folding of booleans casted to integers #2805

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
Merged
merged 37 commits into from
May 23, 2025
Merged

Fix constant folding of booleans casted to integers #2805

merged 37 commits into from
May 23, 2025

Conversation

@annagrin
Copy link
Collaborator

@annagrin annagrin commented Apr 10, 2025 •
edited
Loading

Description

Fix constant folding of casts of integers.

Using the MLIR below

%0 = arith.constant <c> : i8
%1 = cc.cast unsigned %0: i32

The constant folding creates the following code without changing <c>:

%1 = arith.constant <c>: i32

This leads to incorrect values in constants when constant folding is invoked, for example,
(unsigned)(true) is currently converted to a -1 integer constant.

See added tests in targettests/execution/cast.cpp for more examples.

This PR updates constant folding to cast the source of the cast instruction to its original type before folding into a constant of a new type.

Example:

struct testConstBool {
  auto operator()() __qpu__ {
    cudaq::qubit q;
    unsigned i0 = (unsigned)(true);
    if (i0 == 1) {
      x(q);
    }
  }
};

int main() {
  auto counts = cudaq::sample(testConstBool{});
  counts.dump();
}

Quake before the fix:

func.func @__nvqpp__mlirgen__testConstBool () attributes {"cudaq-entrypoint", "cudaq-kernel"} {
    %c-1_i32 = arith.constant -1 : i32
    %c1_i32 = arith.constant 1 : i32
    %0 = quake.alloca !quake.ref
    %1 = cc.alloca i32
    cc.store %c-1_i32, %1 : !cc.ptr<i32>
    %2 = cc.load %1 : !cc.ptr<i32>
    %3 = arith.cmpi eq, %2, %c1_i32 : i32
    cc.if(%3) {
      quake.x %0 : (!quake.ref) -> ()
    }
    return
  }

Quake after the fix:

func.func @__nvqpp__mlirgen__testConstBool () attributes {"cudaq-entrypoint", "cudaq-kernel"} {
    %c1_i32 = arith.constant 1 : i32
    %0 = quake.alloca !quake.ref
    %1 = cc.alloca i32
    cc.store %c1_i32, %1 : !cc.ptr<i32>
    %2 = cc.load %1 : !cc.ptr<i32>
    %3 = arith.cmpi eq, %2, %c1_i32 : i32
    cc.if(%3) {
      quake.x %0 : (!quake.ref) -> ()
    }
    return
  }

Result before the fix:
{ 0:0000 }

Result after the fix:
{ 1:0000 }

Closes: #2804

@annagrin
Fix constant folding of boolean casted to integers
541f2bb 
Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
@github-actions
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Apr 11, 2025
@cuda-quantum-bot
Docs preview for PR #2805.
0d8898c
nvidia-dobri
nvidia-dobri reviewed Apr 11, 2025
View reviewed changes
Copy link
Contributor

@nvidia-dobri nvidia-dobri left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Change looks good, might be useful to know the context of when this is important / what the downstream breakage we saw was.

I'm a bit confused by the statement:

(unsigned)(true) is currently converted to a -1 constant in MLIR

What does -1 in unsigned mean? U..._MAX?

schweitzpgi
schweitzpgi reviewed Apr 12, 2025
View reviewed changes
schweitzpgi
schweitzpgi reviewed Apr 12, 2025
View reviewed changes
annagrin added 3 commits April 14, 2025 10:01
@annagrin
Fix constant folding of boolean casted to integers
48a463c 
Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
@annagrin
Merge branch 'main' of https://github.com/NVIDIA/cuda-quantum into fi…
46a4a7e 
…x-const-folding

Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
@annagrin
Address CR Comments
eafa5e8 
Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
@annagrin
Copy link
Collaborator Author

annagrin commented Apr 14, 2025 •
edited
Loading

Change looks good, might be useful to know the context of when this is important / what the downstream breakage we saw was.

I included an example in the description and the reference to the bug it fixes

I'm a bit confused by the statement:

(unsigned)(true) is currently converted to a -1 constant in MLIR

What does -1 in unsigned mean? U..._MAX?

Constant folding folds the cast, so in this particular example we end up with an integer constant with value of -1, of type i32 (see example in the description). It would behave the same as uint32 max, yes, which is incorrect in this case, we should const fold it to arith.constant 1 instead.

@annagrin
Format
f051dc5 
Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
github-actions bot pushed a commit that referenced this pull request Apr 14, 2025
@cuda-quantum-bot
Docs preview for PR #2805.
a2f7bf0
@github-actions
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

@annagrin
Cleanup test
2daef6c 
Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
github-actions bot pushed a commit that referenced this pull request Apr 14, 2025
@cuda-quantum-bot
Docs preview for PR #2805.
78e754d
@github-actions
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

@annagrin annagrin requested review from 1tnguyen and bmhowe23 April 14, 2025 21:42
annagrin added 2 commits April 17, 2025 11:30
@annagrin
Updated tests
43606b2 
Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
@annagrin
Merge branch 'main' of https://github.com/NVIDIA/cuda-quantum into fi…
a66b338 
…x-const-folding

Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
github-actions bot pushed a commit that referenced this pull request Apr 17, 2025
@cuda-quantum-bot
Docs preview for PR #2805.
bec753b
@github-actions
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

@annagrin
Updated tests
2b4a423 
Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
@github-actions
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request Apr 17, 2025
@cuda-quantum-bot
Docs preview for PR #2805.
f51e111
schweitzpgi
schweitzpgi reviewed Apr 18, 2025
View reviewed changes
github-actions bot pushed a commit that referenced this pull request May 14, 2025
@cuda-quantum-bot
Docs preview for PR #2805.
3fc9058
@github-actions
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

@annagrin
Restart build
87e8ebc 
Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
github-actions bot pushed a commit that referenced this pull request May 15, 2025
@cuda-quantum-bot
Docs preview for PR #2805.
3c40e60
@github-actions
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

schweitzpgi
schweitzpgi reviewed May 22, 2025
View reviewed changes
schweitzpgi
schweitzpgi reviewed May 22, 2025
View reviewed changes
schweitzpgi
schweitzpgi reviewed May 22, 2025
View reviewed changes
schweitzpgi
schweitzpgi reviewed May 22, 2025
View reviewed changes
schweitzpgi
schweitzpgi reviewed May 22, 2025
View reviewed changes
schweitzpgi
schweitzpgi reviewed May 22, 2025
View reviewed changes
github-actions bot pushed a commit that referenced this pull request May 22, 2025
@cuda-quantum-bot
Docs preview for PR #2805.
8e95461
@github-actions
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

@schweitzpgi
Revert some of the unneeded changes to get to a more minimalist
6e8aaa6 
patch. Don't try to preserve NOPs, etc. but fix sign bit truncation
issue for all integral types in the IR.

Signed-off-by: Eric Schweitz <eschweitz@nvidia.com>
schweitzpgi
schweitzpgi approved these changes May 22, 2025
View reviewed changes
Copy link
Collaborator

@schweitzpgi schweitzpgi 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 all the tests!

github-actions bot pushed a commit that referenced this pull request May 22, 2025
@cuda-quantum-bot
Docs preview for PR #2805.
514f81a
@github-actions
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

github-actions bot pushed a commit that referenced this pull request May 23, 2025
@cuda-quantum-bot
Docs preview for PR #2805.
8583521
@github-actions
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

@annagrin annagrin merged commit 0fe1f29 into NVIDIA:main May 23, 2025
195 checks passed
github-actions bot pushed a commit that referenced this pull request May 23, 2025
@cuda-quantum-bot
Cleaning up docs preview for PR #2805.
f3d3481
github-actions bot pushed a commit that referenced this pull request May 23, 2025
@cuda-quantum-bot
Docs preview for PR #2805.
df48bc5
@github-actions
Copy link

CUDA Quantum Docs Bot: A preview of the documentation can be found here.

annagrin added a commit to annagrin/cuda-quantum that referenced this pull request Jun 17, 2025
@annagrin
Fix constant folding of booleans casted to integers (NVIDIA#2805)
c2832ab 
* Fix constant folding of boolean casted to integers

Signed-off-by: Anna Gringauze <agringauze@nvidia.com>

* Fix constant folding of boolean casted to integers

Signed-off-by: Anna Gringauze <agringauze@nvidia.com>

* Address CR Comments

Signed-off-by: Anna Gringauze <agringauze@nvidia.com>

* Format

Signed-off-by: Anna Gringauze <agringauze@nvidia.com>

* Cleanup test

Signed-off-by: Anna Gringauze <agringauze@nvidia.com>

* Updated tests

Signed-off-by: Anna Gringauze <agringauze@nvidia.com>

* Updated tests

Signed-off-by: Anna Gringauze <agringauze@nvidia.com>

* Address CR comments

Signed-off-by: Anna Gringauze <agringauze@nvidia.com>

* Address CR comments

Signed-off-by: Anna Gringauze <agringauze@nvidia.com>

* Cleanup

Signed-off-by: Anna Gringauze <agringauze@nvidia.com>

* Cleanup

Signed-off-by: Anna Gringauze <agringauze@nvidia.com>

* Cleanup

Signed-off-by: Anna Gringauze <agringauze@nvidia.com>

* Cleanup

Signed-off-by: Anna Gringauze <agringauze@nvidia.com>

---------

Signed-off-by: Anna Gringauze <agringauze@nvidia.com>
@bettinaheim bettinaheim added the bug fix To be listed under Bug Fixes in the release notes label Jul 22, 2025
@bettinaheim bettinaheim added this to the release 0.12.0 milestone Jul 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@schweitzpgi schweitzpgi schweitzpgi approved these changes

@nvidia-dobri nvidia-dobri Awaiting requested review from nvidia-dobri

@bmhowe23 bmhowe23 Awaiting requested review from bmhowe23

@1tnguyen 1tnguyen Awaiting requested review from 1tnguyen

Assignees

No one assigned

Labels

bug fix To be listed under Bug Fixes in the release notes

Projects

None yet

Milestone

release 0.12.0

Development

Successfully merging this pull request may close these issues.

Incorrect bool to integer cast

4 participants

@annagrin @schweitzpgi @nvidia-dobri @bettinaheim