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

Python ast bridge and kernel builder do not properly annotate kernels #2541

Open
schweitzpgi opened this issue Jan 24, 2025 · 1 comment
Open
Assignees
Labels
bug Something isn't working python bridge Involves the python bridge to quake

Comments

@schweitzpgi
Copy link
Collaborator

A kernel function must be annotated with the cudaq-kernel attribute. Python doesn't do this making proper identification of kernel functions impossible.

@schweitzpgi schweitzpgi added bug Something isn't working python bridge Involves the python bridge to quake labels Jan 24, 2025
@schweitzpgi schweitzpgi self-assigned this Jan 24, 2025
schweitzpgi added a commit to schweitzpgi/cuda-quantum that referenced this issue Jan 24, 2025
Workaround for issue NVIDIA#2539.

Signed-off-by: Eric Schweitz <[email protected]>
schweitzpgi added a commit to schweitzpgi/cuda-quantum that referenced this issue Jan 28, 2025
Workaround for issue NVIDIA#2539.

Signed-off-by: Eric Schweitz <[email protected]>
schweitzpgi added a commit to schweitzpgi/cuda-quantum that referenced this issue Jan 29, 2025
… codegen for

python. Too many bugs.
Update tests. Fix bugs in mock servers. Have python kernel builder add the
cudaq-kernel attribute. (See issue 2541.)

Signed-off-by: Eric Schweitz <[email protected]>
schweitzpgi added a commit to schweitzpgi/cuda-quantum that referenced this issue Jan 29, 2025
shows its age at this point. It was written before much of the rest
of the core was implemented or even designed.

These changes replace the existing QIR codegen for the C++ compiler.
Unfortunately, there are a number of semantics violations from Python
that need to be fixed before it can be switched over. Therefore, this
PR leaves Python intact and using the old codegen passes.

The purpose of these changes is two-fold.

 1. Instead of converting all of the IR to the LLVM-IR dialect at
    the same time as converting to QIR, these two steps are bifurcated.
    This allows us to convert the quake dialect to other higher level
    operations first and before LLVM-IR dialect is introduced. This
    will be beneficial in short order...
 2. Instead of piecemealing different flavors of QIR in completely ad
    hoc spaghetti plate passes, the flavor of QIR is specified as a
    mixin modifier class for a singular set of steps to convert to any
    flavor of QIR. This does mean that one will no longer be able to
    convert to the LLVM-IR dialect with QIR calls and then change their
    mind from chocolate QIR to strawberry QIR much later.

Notes:
 - Remove the option to disable the qir profile preparation pass.
   This pass is not optional. The IR will be translated to an
   invalid state if the required function declarations are not
   created at all.
 - Make it clear that AggressiveEarlyInlining is a pipeline. Refactor
   the registration functions so that we're not block-copying things
   between the core and python (which was dropping things on the floor
   already).
 - Add a new pass, convert to QIR API.
   This pass will replace the cascade of passes to convert to full QIR and
   then convert some more to base profile or adaptive profile.
 - Refactor QIRFunctionNames.h.
 - Add a raft of declarations to the intrinsics table. This will
   dramatically reduce the amount of code in codegen and make
   maintenance much easier.
 - Add the analysis and prep pass.
 - Improve pipeline locality and performance.
 - Use the new code in the default code gen path for C++.
 - Workarounds for issue NVIDIA#2541 and issue NVIDIA#2539. Keep the old codegen for
   python. Too many bugs.
 - Update tests. Fix bugs in mock servers. Have python kernel builder add the
   cudaq-kernel attribute. (See issue 2541.)

Signed-off-by: Eric Schweitz <[email protected]>
@schweitzpgi
Copy link
Collaborator Author

PR #2542 has at least a partial fix for this bug.

schweitzpgi added a commit to schweitzpgi/cuda-quantum that referenced this issue Feb 3, 2025
shows its age at this point. It was written before much of the rest
of the core was implemented or even designed.

These changes replace the existing QIR codegen for the C++ compiler.
Unfortunately, there are a number of semantics violations from Python
that need to be fixed before it can be switched over. Therefore, this
PR leaves Python intact and using the old codegen passes.

The purpose of these changes is two-fold.

 1. Instead of converting all of the IR to the LLVM-IR dialect at
    the same time as converting to QIR, these two steps are bifurcated.
    This allows us to convert the quake dialect to other higher level
    operations first and before LLVM-IR dialect is introduced. This
    will be beneficial in short order...
 2. Instead of piecemealing different flavors of QIR in completely ad
    hoc spaghetti plate passes, the flavor of QIR is specified as a
    mixin modifier class for a singular set of steps to convert to any
    flavor of QIR. This does mean that one will no longer be able to
    convert to the LLVM-IR dialect with QIR calls and then change their
    mind from chocolate QIR to strawberry QIR much later.

Notes:
 - Remove the option to disable the qir profile preparation pass.
   This pass is not optional. The IR will be translated to an
   invalid state if the required function declarations are not
   created at all.
 - Make it clear that AggressiveEarlyInlining is a pipeline. Refactor
   the registration functions so that we're not block-copying things
   between the core and python (which was dropping things on the floor
   already).
 - Add a new pass, convert to QIR API.
   This pass will replace the cascade of passes to convert to full QIR and
   then convert some more to base profile or adaptive profile.
 - Refactor QIRFunctionNames.h.
 - Add a raft of declarations to the intrinsics table. This will
   dramatically reduce the amount of code in codegen and make
   maintenance much easier.
 - Add the analysis and prep pass.
 - Improve pipeline locality and performance.
 - Use the new code in the default code gen path for C++.
 - Workarounds for issue NVIDIA#2541 and issue NVIDIA#2539. Keep the old codegen for
   python. Too many bugs.
 - Update tests. Fix bugs in mock servers. Have python kernel builder add the
   cudaq-kernel attribute. (See issue 2541.)

Signed-off-by: Eric Schweitz <[email protected]>
schweitzpgi added a commit that referenced this issue Feb 3, 2025
* The QIR codegen is one of the oldest pieces of the project and really
shows its age at this point. It was written before much of the rest
of the core was implemented or even designed.

These changes replace the existing QIR codegen for the C++ compiler.
Unfortunately, there are a number of semantics violations from Python
that need to be fixed before it can be switched over. Therefore, this
PR leaves Python intact and using the old codegen passes.

The purpose of these changes is two-fold.

 1. Instead of converting all of the IR to the LLVM-IR dialect at
    the same time as converting to QIR, these two steps are bifurcated.
    This allows us to convert the quake dialect to other higher level
    operations first and before LLVM-IR dialect is introduced. This
    will be beneficial in short order...
 2. Instead of piecemealing different flavors of QIR in completely ad
    hoc spaghetti plate passes, the flavor of QIR is specified as a
    mixin modifier class for a singular set of steps to convert to any
    flavor of QIR. This does mean that one will no longer be able to
    convert to the LLVM-IR dialect with QIR calls and then change their
    mind from chocolate QIR to strawberry QIR much later.

Notes:
 - Remove the option to disable the qir profile preparation pass.
   This pass is not optional. The IR will be translated to an
   invalid state if the required function declarations are not
   created at all.
 - Make it clear that AggressiveEarlyInlining is a pipeline. Refactor
   the registration functions so that we're not block-copying things
   between the core and python (which was dropping things on the floor
   already).
 - Add a new pass, convert to QIR API.
   This pass will replace the cascade of passes to convert to full QIR and
   then convert some more to base profile or adaptive profile.
 - Refactor QIRFunctionNames.h.
 - Add a raft of declarations to the intrinsics table. This will
   dramatically reduce the amount of code in codegen and make
   maintenance much easier.
 - Add the analysis and prep pass.
 - Improve pipeline locality and performance.
 - Use the new code in the default code gen path for C++.
 - Workarounds for issue #2541 and issue #2539. Keep the old codegen for
   python. Too many bugs.
 - Update tests. Fix bugs in mock servers. Have python kernel builder add the
   cudaq-kernel attribute. (See issue 2541.)

Signed-off-by: Eric Schweitz <[email protected]>

---------

Signed-off-by: Eric Schweitz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working python bridge Involves the python bridge to quake
Projects
None yet
Development

No branches or pull requests

1 participant