CMake 3.21 rc-1 | HIP support breaks custom MSVC toolchain

Oh, I see now. I wonder if MSVC supports short-circuiting __has_include. It would seem not, so maybe just moving the && __has_include inside of the __clang__ detection would suffice.

It appears to be an issue with MSVC. It’s skipping the __clang__ part.

My guess is that the parser is just greedy and tries to evaluate __has_include before considering that it’s unreachable due to other boolean logic. A bug in MSVC (that should be reported), but something we should still work around.

There is code here that indicates the HIP compiler can simulate the MSVC host ABI on Windows. Therefore we may not be able to just use !defined(_MSC_VER).

No we aren’t we are just using regular MSVC. We just maintain the binaries in a specific location so that updating Visual Studio doesn’t update our compilers.

If that’s the case than what about my proposed idea of just using a dummy include directory. Since that also fixes the issue.

It seems to me that __has_include just didn’t account for users not providing any include directory at all.

cl.exe foobar.cpp # Fails
cl.exe CMake/foobar.cpp /I:CMake/ # Passes

what about my proposed idea of just using a dummy include directory.

This code is being compiled in order to identify the compiler, so we don’t yet know that it is MSVC or capable of accepting a /I flag.

maybe just moving the && __has_include inside of the __clang__ detection would suffice.

That should work. It may be tricky to get the compiler id code generated that way. I’ll take a look.

Actually we do have code already to try various flags known to be needed for certain toolchains. Adding that is easier than changing the way the compiler id code generation works (because it assumes a cascading #elif sequence).

See CMake MR 6284 for a fix. It adds a fallback attempt to identify the compiler by passing -I__does_not_exist__.

1 Like

Is this legal syntax for the MSVC compiler?

-I__does_not_exist__

Yes. MS-style command lines accept both / and - for options. The -I (or /I) option accepts the include directory immediately or in a following argument.

I was able to reproduce the error by setting my INCLUDE environment variable to empty, and then verify that my change fixes it.

Sweet :+1: thank you very much for the quick turn around.

EDIT:
I just tested your idea locally as well to confirm it works. :+1:

Apologies turns out RC2 didn’t fix my issue. I’m still able to reproduce the issue.

-I__does_not_exist__ isn’t being used during the compilation of the CMakeCXXCompilerId.cpp so it still fails.

EDIT:

I can fix the issue locally by doing this as you pointed out @brad.king:

image

I’m looking at the merge request, and I’m confused what CMAKE_C_COMPILER_ID_TEST_FLAGS does?

CMAKE_C_COMPILER_ID_TEST_FLAGS is a list of flags known to be needed by various compilers to produce a file that the compiler id logic can use. We try first with no flags, and then with each of the flags in that list until one works. Check CMakeFiles/CMakeError.log to see all the failures, and CMakeFiles/CMakeOutput.log to see the one that worked, if any.

-I__does_not_exist__ isn’t being used during the compilation of the CMakeCXXCompilerId.cpp so it still fails.

It’s expected that a few attempts will be made without the flag, because prior to knowing the compiler id all we can do is guess flag combinations until one works.

FWIW, I did test this locally by setting an empty INCLUDE in my environment. With that I could reproduce the error, and then see the fix work. CMakeFiles/CMakeError.log ends up with a bunch of failed attempts before the one with -I__does_not_exist__ works.

Please try configuring in a new build tree with --trace-expand 2>log, check the log for any private details, and then attach it here.

It seems to fail on the linker step… but this didn’t used to matter.

I just used a raw command line myself to test it out.

D:/.../vc/14.28.29910/bin/Hostx64/x64/cl.exe D:\...\build\ninja\wnow64\CMakeFiles\3.21.0-rc2\CompilerIdCXX\CMakeCXXCompilerId.cpp -I__does_not_exist
Microsoft (R) C/C++ Optimizing Compiler Version 19.28.29910 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

CMakeCXXCompilerId.cpp
Microsoft (R) Incremental Linker Version 14.28.29910.0    
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:CMakeCXXCompilerId.exe 
CMakeCXXCompilerId.obj 
LINK : fatal error LNK1104: cannot open file 'LIBCMT.lib'

Hmm. I wonder if it was working with the -c flag in that list before.

Try editing Modules/CMakeDetermineCXXCompiler.cmake locally with this patch:

-    "-I__does_not_exist__"
+    "-c -I__does_not_exist__"

The “-c” + “-I__does_not_exist__” combination works perfectly!

PS D:\...\foobar> d:/.../vc/14.28.29910/bin/Hostx64/x64/cl.exe D:\...\build\ninja\wnow64\CMakeFiles\3.21.0-rc2\CompilerIdCXX\CMakeCXXCompilerId.cpp -I__does_not_exist -c
Microsoft (R) C/C++ Optimizing Compiler Version 19.28.29910 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

CMakeCXXCompilerId.cpp
PS D:\...\foobar>

Great. I’ll update the fix.

See CMake MR 6295. I was able to reproduce the compiler id linking failure by setting both INCLUDE and LIB to empty values, and also verify that the (new) fix works.

1 Like

Btw @brad.king I’d like to report this compiler bug to MSVC. What is the proper channel to do so?

I reported it through the VS Developer Community feedback link (“Report a problem”):

2 Likes

The VS 2022 preview 4 compiler cl version 19.30.30528 fixed __has_include to work when there are no include directories.

2 Likes