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

nvq++ bridge compilation issue with std::vector<bool> argument #1383

Closed
3 of 4 tasks
1tnguyen opened this issue Mar 13, 2024 · 0 comments · Fixed by #1402
Closed
3 of 4 tasks

nvq++ bridge compilation issue with std::vector<bool> argument #1383

1tnguyen opened this issue Mar 13, 2024 · 0 comments · Fixed by #1402
Assignees

Comments

@1tnguyen
Copy link
Collaborator

Required prerequisites

  • Consult the security policy. If reporting a security vulnerability, do not report the bug using this form. Use the process described in the policy to report the issue.
  • Make sure you've read the documentation. Your issue may be addressed there.
  • Search the issue tracker to verify that this hasn't already been reported. +1 or comment there if it has.
  • If possible, make a PR with a failing test to give us a starting point to work on!

Describe the bug

Compilation error: 'cc.if' op operand #0 must be 1-bit signless integer, but got '!cc.ptr<i1>' when dereferencing a std::vector<bool> argument in MLIR mode.

Steps to reproduce the bug

#include <cudaq.h>

void f(const std::vector<bool>& k) __qpu__ {
  cudaq::qubit q;
  for (int i = 0; i < k.size(); ++i)
    if (k[i])
        x(q);
}

int main() {
  std::vector<bool> x { true, false};  
  auto counts = cudaq::sample(f, x);
  counts.dump();
  return 0;
}

The above example works in library mode but fails in MLIR mode:

error: 'cc.if' op operand #0 must be 1-bit signless integer, but got '!cc.ptr<i1>'
"builtin.module"() ({
  "func.func"() ({
  ^bb0(%arg0: !cc.stdvec<i1>):
    %0 = "quake.alloca"() : () -> !quake.ref
    "cc.scope"() ({
      %1 = "arith.constant"() {value = 0 : i32} : () -> i32
      %2 = "cc.alloca"() {elementType = i32} : () -> !cc.ptr<i32>
      "cc.store"(%1, %2) : (i32, !cc.ptr<i32>) -> ()
      "cc.loop"() ({
        %3 = "cc.load"(%2) : (!cc.ptr<i32>) -> i32
        %4 = "arith.extsi"(%3) : (i32) -> i64
        %5 = "cc.stdvec_size"(%arg0) : (!cc.stdvec<i1>) -> i64
        %6 = "arith.cmpi"(%4, %5) {predicate = 6 : i64} : (i64, i64) -> i1
        "cc.condition"(%6) : (i1) -> ()
      }, {
        %3 = "func.constant"() {value = @_ZNKSt6vectorIbSaIbEEixEm} : () -> ((i64) -> i1)
        %4 = "cc.load"(%2) : (!cc.ptr<i32>) -> i32
        %5 = "arith.extsi"(%4) : (i32) -> i64
        %6 = "cc.stdvec_data"(%arg0) : (!cc.stdvec<i1>) -> !cc.ptr<i1>
        %7 = "cc.compute_ptr"(%6, %5) {rawConstantIndices = array<i32: -2147483648>} : (!cc.ptr<i1>, i64) -> !cc.ptr<i1>
        "cc.if"(%7) ({
          %8 = "func.constant"() {value = @_ZN5cudaq1xINS_4baseEJNS_5quditILm2EEEEEEvDpRT0_} : () -> ((!quake.ref) -> ())
          "quake.x"(%0) {operand_segment_sizes = array<i32: 0, 0, 1>} : (!quake.ref) -> ()
          "cc.continue"() : () -> ()
        }, {
        }) : (!cc.ptr<i1>) -> ()
        "cc.continue"() : () -> ()
      }, {
        %3 = "cc.load"(%2) : (!cc.ptr<i32>) -> i32
        %4 = "arith.constant"() {value = 1 : i32} : () -> i32
        %5 = "arith.addi"(%3, %4) : (i32, i32) -> i32
        "cc.store"(%5, %2) : (i32, !cc.ptr<i32>) -> ()
        "cc.continue"() : () -> ()
      }) {post_condition = false} : () -> ()
      "cc.continue"() : () -> ()
    }) : () -> ()
    "func.return"() : () -> ()
  }) {"cudaq-entrypoint", "cudaq-kernel", function_type = (!cc.stdvec<i1>) -> (), no_this, sym_name = "__nvqpp__mlirgen__function_f._Z1fRKSt6vectorIbSaIbEE"} : () -> ()
  "func.func"() ({
  }) {function_type = (i64) -> i1, sym_name = "_ZNKSt6vectorIbSaIbEEixEm", sym_visibility = "private"} : () -> ()
  "func.func"() ({
  }) {function_type = (!quake.ref) -> (), sym_name = "_ZN5cudaq1xINS_4baseEJNS_5quditILm2EEEEEEvDpRT0_", sym_visibility = "private"} : () -> ()
  "func.func"() ({
  ^bb0(%arg0: !cc.ptr<!cc.struct<{!cc.ptr<i1>, !cc.ptr<i1>, !cc.ptr<i1>}>>):
    "func.return"() : () -> ()
  }) {function_type = (!cc.ptr<!cc.struct<{!cc.ptr<i1>, !cc.ptr<i1>, !cc.ptr<i1>}>>) -> (), no_this, sym_name = "_Z1fRKSt6vectorIbSaIbEE"} : () -> ()
}) {llvm.data_layout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128", llvm.triple = "x86_64-unknown-linux-gnu", quake.mangled_name_map = {__nvqpp__mlirgen__function_f._Z1fRKSt6vectorIbSaIbEE = "_Z1fRKSt6vectorIbSaIbEE"}} : () -> ()
Passes failed!

Expected behavior

No compile errors.

Is this a regression? If it is, put the last known working version (or commit) here.

Not a regression

Environment

  • CUDA Quantum version: main
  • Python version: 3.10
  • C++ compiler: gcc-12
  • Operating system: Ubuntu 22.04

Suggestions

No response

@schweitzpgi schweitzpgi self-assigned this Mar 13, 2024
schweitzpgi added a commit to schweitzpgi/cuda-quantum that referenced this issue Mar 14, 2024
The operator[] returns a reference, but the AST doesn't have a cast
node. So the value appearing in the conditional is not loaded.
This adds an explicit check and an error message if the load is
returning an unexpected value.

Fix NVIDIA#1383.
schweitzpgi added a commit that referenced this issue Mar 14, 2024
The operator[] returns a reference, but the AST doesn't have a cast
node. So the value appearing in the conditional is not loaded.
This adds an explicit check and an error message if the load is
returning an unexpected value.

Fix #1383.
anthony-santana added a commit that referenced this issue Mar 15, 2024
* Remove exact version specifier in Python install instructions (#1373)

Anaconda doesn't have linux-aarch64 packages for Python 3.8.0 and 3.9.0,
so without this change, these instructions will fail on ARM. However,
when switching to a single "=", conda will allow the user to specify a
version up to the number of digits provided, and then use the latest
version available after that. For example, if the user provides "3.8",
then conda will choose "3.8.18", which is the largest patch version
matching "3.8".

* Python readme update (#1378)

* Fix a bug in sample.h affecting sample_async in library mode (#1379)

In library mode, we need to run a tracing pass before actual execution.
With multi-qpu, we set_exec_ctx on specific qpu id but didn't reset the
context on that particular qpu id (fall back to reset QPU 0 context all
the time).

* Fix bug - enable list[T] argument specification with kernel builder (#1370)

* Enable list[T] specification with kernel builder.

Signed-off-by: Alex McCaskey <amccaskey@nvidia.com>

* small fixes

Signed-off-by: Alex McCaskey <amccaskey@nvidia.com>

---------

Signed-off-by: Alex McCaskey <amccaskey@nvidia.com>

* * Add docstrings for 'draw' (#1377)

* Make nvqc_proxy.py multi-threaded (#1387)

Full backstory and test details are in PR.

* Fix a few Python notebook examples for release (#1386)

* Fix #1348 - synthesis of vectors with small integers. (#1395)

The constant array op is using at ArrayAttr, which does not naturally
support integral sizes less than i32. This patch upcasts the short
values to i32 to get around the issue. Another approach would be to
have the constant array op take a DenseArrayAttr, which does support
the shorter integral types. Might need a custom parser/pretty printer
if we go that route?

* [test] Fix deployment issue with #1370 (#1394)

* Fix #1383: bridge not lowering to correct IR (#1402)

The operator[] returns a reference, but the AST doesn't have a cast
node. So the value appearing in the conditional is not loaded.
This adds an explicit check and an error message if the load is
returning an unexpected value.

Fix #1383.

* Fix #1392: "syntehsis" of callables (#1398)

Refactor the code that was to catch struct value passing (which is
not yet implemented) from arguments that are callables.

Fix #1392

* Path fix in publishing (#1399)

---------

Signed-off-by: Alex McCaskey <amccaskey@nvidia.com>
Co-authored-by: Ben Howe <141149032+bmhowe23@users.noreply.github.com>
Co-authored-by: Bettina Heim <heimb@outlook.com>
Co-authored-by: Thien Nguyen <58006629+1tnguyen@users.noreply.github.com>
Co-authored-by: Alex McCaskey <amccaskey@nvidia.com>
Co-authored-by: Pradnya Khalate <148914294+khalatepradnya@users.noreply.github.com>
Co-authored-by: Eric Schweitz <eschweitz@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants