[c++17] changes to cudaq headers for pure c++17 environment

[schweitzpgi]

The driver and headers were allowing all C++20 syntax to pass, even in C++17 mode. This was happening by leaning into the idea that a C++20 compiler was available, it was just being told to compile as C++17. This meant that in a pure C++17 environment, which didn't know anything about C++20 whatsoever, would choke on the CUDA-Q syntax.

This PR disables the use of the offending syntax in the headers unless a C++20 (or better) compiler is truly in use. This distinction means that CUDA-Q programs written for C++17 may be required to use a different syntax in order to compile.

For example, the following C++20 syntax

  // C++20 only!
  void qernel(cudaq::qubit &c, cudaq::qubit &q) __qpu__ {
    x<cudaq::ctrl>(c, q);
  }

must be written as

  // C++17 or C++20
  void qernel(cudaq::qubit &c, cudaq::qubit &q) __qpu__ {
    cx(c, q);
  }

[1tnguyen]

LGTM.

I also want to ask @bmhowe23's opinion regarding breaking/non-breaking classification of the syntax change from x<ctrl>(a,b,c,d) to cx(a,b,c,d), etc. when compiling with -std=c++17.

[bettinaheim]

@schweitzpgi What's the issue with the reasoning that the nvq++ compiler is necessarily used to compile the code and nvq++ contains C++20 support? Is there a scenario where the code with nvq++ --std=c++17 would not compile or run when only a C++17 runtime is available?

If we pull in this change, we should remove the -Wno-c++20-extensions flag here and here.

[amccaskey]

LGTM.

I also want to ask @bmhowe23's opinion regarding breaking/non-breaking classification of the syntax change from x<ctrl>(a,b,c,d) to cx(a,b,c,d), etc. when compiling with -std=c++17.

traditionally, cOP has meant the singly-controlled version of OP. Then we'd see Toffoli gates (doubly-controlled) expressed as ccx. This change departs from this and allows the full OP<ctrl>(...) functionality for multi-controlled operations. I do not think we should do this since it departs from a norm. If a C++17 user wants a general multi-control operations on more than 1 control, they should use cudaq::control(...) on a kernel calling the OP.

[bmhowe23]

@schweitzpgi What's the issue with the reasoning that the nvq++ compiler is necessarily used to compile the code and nvq++ contains C++20 support? Is there a scenario where the code with nvq++ --std=c++17 would not compile or run when only a C++17 runtime is available?

I assume the requirement that Eric is trying to satisfy is that cudaq.h, which includes runtime/cudaq/qis/qubit_qis.h, should be includable by any C++17 compiler (e.g. nvcc). However, I think this is already achievable.

For example, this file:

#include "cudaq.h"
int test() {
    return 0;
}

is compilable with all 3 of these commands.

$ nvcc -c -I/usr/local/cudaq/include -std=c++17 bentest.cpp -o bentest.o
$ g++ -c -I/usr/local/cudaq/include -std=c++17 bentest.cpp -o bentest.o
$ /opt/llvm/bin/clang++ -c -I/usr/local/cudaq/include -std=c++17 bentest.cpp -o bentest.o

Is the requirement that our full language is callable from C++17 code, or just that the standard cudaq.h header files cannot interfere with a C++17-only compiler?

[schweitzpgi]

@schweitzpgi What's the issue with the reasoning that the nvq++ compiler is necessarily used to compile the code and nvq++ contains C++20 support?

The bug in the current setup is that we simply allow code that is not C++17 to compile without so much as a warning as C++17. That can only happen if the compiler already knows about C++20. This is clearly problematic. For one thing, it lets us assume we can write tests and other code which is incorrect and no amount of testing will catch the problems.

In library mode, it should not the case that we force users down specific usage paths because we can't be consistent and do the right thing.

[schweitzpgi]

LGTM.
I also want to ask @bmhowe23's opinion regarding breaking/non-breaking classification of the syntax change from x<ctrl>(a,b,c,d) to cx(a,b,c,d), etc. when compiling with -std=c++17.

traditionally, cOP has meant the singly-controlled version of OP. Then we'd see Toffoli gates (doubly-controlled) expressed as ccx. This change departs from this and allows the full OP<ctrl>(...) functionality for multi-controlled operations. I do not think we should do this since it departs from a norm. If a C++17 user wants a general multi-control operations on more than 1 control, they should use cudaq::control(...) on a kernel calling the OP.

Yes, this was a bit of a backdoor extension. I can remove it. Thanks for the feedback.

[schweitzpgi]

If we pull in this change, we should remove the -Wno-c++20-extensions flag here and here.

Taken care of in c16e5ff

[schweitzpgi]

@schweitzpgi What's the issue with the reasoning that the nvq++ compiler is necessarily used to compile the code and nvq++ contains C++20 support? Is there a scenario where the code with nvq++ --std=c++17 would not compile or run when only a C++17 runtime is available?

I assume the requirement that Eric is trying to satisfy is that cudaq.h, which includes runtime/cudaq/qis/qubit_qis.h, should be includable by any C++17 compiler (e.g. nvcc). However, I think this is already achievable.

For example, this file:

#include "cudaq.h"
int test() {
    return 0;
}

is compilable with all 3 of these commands.

$ nvcc -c -I/usr/local/cudaq/include -std=c++17 bentest.cpp -o bentest.o
$ g++ -c -I/usr/local/cudaq/include -std=c++17 bentest.cpp -o bentest.o
$ /opt/llvm/bin/clang++ -c -I/usr/local/cudaq/include -std=c++17 bentest.cpp -o bentest.o

Is the requirement that our full language is callable from C++17 code, or just that the standard cudaq.h header files cannot interfere with a C++17-only compiler?

Yes, it should be true that the cuda.h file is (and should remain) legal C++17 and should compile with any C++17 compiler. That was the requirement: supporting C++17 only systems. We don't want to silently regress from this point, which is possible because we are disabling our compiler checks currently as a workaround for an omission.

Thanks for verifying.

[bmhowe23]

@schweitzpgi What's the issue with the reasoning that the nvq++ compiler is necessarily used to compile the code and nvq++ contains C++20 support? Is there a scenario where the code with nvq++ --std=c++17 would not compile or run when only a C++17 runtime is available?

I assume the requirement that Eric is trying to satisfy is that cudaq.h, which includes runtime/cudaq/qis/qubit_qis.h, should be includable by any C++17 compiler (e.g. nvcc). However, I think this is already achievable.
For example, this file:

#include "cudaq.h"
int test() {
    return 0;
}

is compilable with all 3 of these commands.

$ nvcc -c -I/usr/local/cudaq/include -std=c++17 bentest.cpp -o bentest.o
$ g++ -c -I/usr/local/cudaq/include -std=c++17 bentest.cpp -o bentest.o
$ /opt/llvm/bin/clang++ -c -I/usr/local/cudaq/include -std=c++17 bentest.cpp -o bentest.o

Is the requirement that our full language is callable from C++17 code, or just that the standard cudaq.h header files cannot interfere with a C++17-only compiler?

Yes, it should be true that the cuda.h file is (and should remain) legal C++17 and should compile with any C++17 compiler. That was the requirement: supporting C++17 only systems. We don't want to silently regress from this point, which is possible because we are disabling our compiler checks currently as a workaround for an omission.

Thanks for verifying.

To be clear, the above test file and test compile commands worked for me without any of the changes in this PR. Were you seeing failures before this PR with a specific compiler, or was this issue found by inspection only?

[schweitzpgi]

To be clear, the above test file and test compile commands worked for me without any of the changes in this PR. Were you seeing failures before this PR with a specific compiler, or was this issue found by inspection only?

This was found by inspection. The current setup accepts C++20 programs even when adding the C++17 flag, which is a problem.

[github-actions]

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