[SYCL][NFC] Use /clang:-std= instead of /std: to set C++ standard

[0x12CC]

The two options are similar but /clang:-std= works for C++ 23 on Windows.

[0x12CC]

@uditagarwal97, friendly ping.

[uditagarwal97]

@uditagarwal97, friendly ping.

Hi,
Thanks for the reminder. I've got the following two questions:

The two options are similar but /Qstd works for C++ 23 on Windows.

1. Doesn't /std work for C++ 23 on Windows? IIUC, both of these options should be semantically equivalent.
2. (Just thinking out loud) /std was added for compatibility with MSVC (https://www.intel.com/content/www/us/en/docs/cpp-compiler/developer-guide-reference/2021-8/std-qstd.html), won't changing /std to /Qstd in LIT be erroneous when the host compiler is MSVC (if some test is using a different host and device compiler).

[0x12CC]

Thanks for taking a look at this.

1. Doesn't /std work for C++ 23 on Windows? IIUC, both of these options should be semantically equivalent.

No, it doesn't seem to work. With C++ 23, the driver produces the following error:

icx: error: argument unused during compilation: '-std:c++23

According to the MSVC documentation, the correct flag would be /std:c++23preview. I can't use this flag in a test case since it doesn't work on Linux. Changing /std to /Qstd enables the test case to use c++23 for both Linux and Windows.

2. (Just thinking out loud) /std was added for compatibility with MSVC (https://www.intel.com/content/www/us/en/docs/cpp-compiler/developer-guide-reference/2021-8/std-qstd.html), won't changing /std to /Qstd in LIT be erroneous when the host compiler is MSVC (if some test is using a different host and device compiler).

I understood that the differences in the DPC++ driver are only related to new versions of C++ like c++23preview and c++latest. Any LIT test that was passing with the DPC++ driver should not be impacted by this change. I don't believe we have any LIT tests that use the MSVC driver.

[uditagarwal97]

I can't use this flag in a test case since it doesn't work on Linux.

I see. I'm curious about why the test case needs C++23, when it's still in "preview"?

[uditagarwal97]

Thanks for taking a look at this.

1. Doesn't /std work for C++ 23 on Windows? IIUC, both of these options should be semantically equivalent.

No, it doesn't seem to work. With C++ 23, the driver produces the following error:

icx: error: argument unused during compilation: '-std:c++23

According to the MSVC documentation, the correct flag would be /std:c++23preview. I can't use this flag in a test case since it doesn't work on Linux. Changing /std to /Qstd enables the test case to use c++23 for both Linux and Windows.

2. (Just thinking out loud) /std was added for compatibility with MSVC (https://www.intel.com/content/www/us/en/docs/cpp-compiler/developer-guide-reference/2021-8/std-qstd.html), won't changing /std to /Qstd in LIT be erroneous when the host compiler is MSVC (if some test is using a different host and device compiler).

I understood that the differences in the DPC++ driver are only related to new versions of C++ like c++23preview and c++latest. Any LIT test that was passing with the DPC++ driver should not be impacted by this change. I don't believe we have any LIT tests that use the MSVC driver.

@mdtoguchi FYI. I'm not sure if we should consider it as a bug in clang driver. Even if we were to workaround this problem by mapping /std=c++23 to /std=c++23preview on windows (with MSVC), it would be difficult to maintain this workaround when C++23 is fully supported by MSVC or with newer C++ versions. Nevertheless, if not a bug, we should atleast document this driver behavior.

[0x12CC]

I see. I'm curious about why the test case needs C++23, when it's still in "preview"?

The relevant PRs are here:

1. Add sycl_khr_group_interface extension KhronosGroup/SYCL-Docs#638
2. Implement the sycl_khr_group_interface extension #17595

The KHR extension includes member functions that require features from C++ 23. These features are fully supported in both Clang and DPC++.

@mdtoguchi FYI. I'm not sure if we should consider it as a bug in clang driver. Even if we were to workaround this problem by mapping /std=c++23 to /std=c++23preview on windows (with MSVC), it would be difficult to maintain this workaround when C++23 is fully supported by MSVC or with newer C++ versions. Nevertheless, if not a bug, we should atleast document this driver behavior.

I think the relevant mapping in the driver is here:

llvm/clang/lib/Driver/ToolChains/Clang.cpp

Line 8216 in 7278476

.Case("c++23preview", "-std=c++23")

I don't believe there's a bug and we shouldn't need to maintain any additional mapping. My change just uses the version of the flag that's not MSVC-compatible so that LIT tests can specify the same standard on both Windows and Linux. I'm open to using an alternative approach if you think there's a better way to do this.

[uditagarwal97]

Thanks for providing the additional context @0x12CC!
Change LGTM.

[uditagarwal97]

Windows CI failure should be unrelated. I've created a PR to fix it: #18939. Restarting Windows CI tests