Skip to content

Conversation

@illwieckz
Copy link
Member

@illwieckz illwieckz commented Dec 3, 2023

Those are old commits that were sitting on my hard drive for a long time. I believe I wrote them when we were investigating the console bug that only happened on clang and clang-based compilers. I assume that enabling PCH with Saigo would now just require to replace the test for NACL with a test for the PNaCl compiler.

The first need I wanted to fulfill was to write in daemon.log the compiler that was used to build the game, in order to give useful information to people dealing with bug reports.

The second need I wanted to fulfill was to know at build time by reading the build log what CMake was really using when I set some options.

While I was at it, I added custom code to recognize custom compilers like PNaCl and Saigo (in fact at the time I had added Wasi recognition but I removed that before pushing because we really don't know yet which compiler we will use for Wasm).

I also added recognition and compatibility with some other compilers I usually use to test the game, which usually catches different warnings in ways that can be useful to debug things or spot errors. This includes Intel ICC and ICX, AMD AOCC, and Zig. This is not meant to add “support” in a way we would claim to support this, we actually don't have to advertise the fact those compilers can build the game, and this does not give us any obligation to fix the compatibility if one day it breaks. This is only useful for testing, the same way I test the game on various CPU and GPU, I test the build on many compilers.

While doing this I spotted some shortcomings like some flags not being the same for the engine if built within Unvanquished repository or from Daemon repository.

I also spotted that we configure engine platform, compiler even when we don't build the engine, even when we will use another compiler for building the vms.

For those two shortcomings I only added comments and some print to tell when some CMake logs are meaningless, because fixing those shortcomings that exist in our CMake code since forever is out of topic of this PR.

If the compiler is unknown, it reports Unknown as compiler name and Unknown as compiler version. It is expected the clang version is appended if the unknown compiler is clang-based (like known ones).

See also:

@illwieckz
Copy link
Member Author

illwieckz commented Dec 3, 2023

When building we get logs like that:

-- Detected architecture: amd64
-- CMake generator: Unix Makefiles
-- Detected C compiler: GNU 13.2.0
-- Detected C++ compiler: GNU 13.2.0
-- Detected architecture: amd64
-- CMake generator: Unix Makefiles
-- Detected C compiler: Clang 16.0.6
-- Detected C++ compiler: Clang 16.0.6
-- Detected architecture: amd64
-- CMake generator: Unix Makefiles
-- Detected C compiler: Intel 2021.10.0.20230609
-- Detected C++ compiler: Intel 2021.10.0.20230609
-- Detected architecture: amd64
-- CMake generator: Unix Makefiles
-- Detected C compiler: IntelLLVM 2024.0.0/clang-17.0.0
-- Detected C++ compiler: IntelLLVM 2024.0.0/clang-17.0.0
-- Detected architecture: amd64
-- CMake generator: Unix Makefiles
-- Detected C compiler: AOCC 4.0.0-Build#434/clang-14.0.6
-- Detected C++ compiler: AOCC 4.0.0-Build#434/clang-14.0.6
-- Detected architecture: pnacl
-- CMake generator: Unix Makefiles
-- Detected C compiler: PNaCl 4.2.1/clang-3.6.0
-- Detected C++ compiler: PNaCl 4.2.1/clang-3.6.0
-- Detected architecture: amd64
-- CMake generator: Unix Makefiles
-- Detected C compiler: AppleClang 13.0.0.13000029/clang-13.0.0 
-- Detected C++ compiler: AppleClang 13.0.0.13000029/clang-13.0.0 
-- Detected architecture: amd64
-- CMake generator: Visual Studio 16 2019
-- Detected C compiler: MSVC 19.29.30153.0 
-- Detected C++ compiler: MSVC 19.29.30153.0

When running the game we get logs like that:

Unvanquished 0.54.0 Linux amd64 AOCC 4.0.0-Build~434/clang-14.0.6 Dec  3 2023
SGame Native Client pnacl PNaCl 4.2.1/clang-3.6.0 Dec  3 2023
CGame Native Client pnacl PNaCl 4.2.1/clang-3.6.0 Dec  3 2023

@illwieckz
Copy link
Member Author

Actually this don't detect MingW as a variant of GCC:

-- Detected architecture: amd64
-- CMake generator: Ninja
-- Detected C compiler: GNU 9.3.0 
-- Detected C++ compiler: GNU 9.3.0

This may be improved, but this is a good example of how the code is assumed to be safe: in worst case there is less information printed, that's all.

@illwieckz illwieckz force-pushed the illwieckz/compiler-string/sync branch 4 times, most recently from 9385ec6 to 9effab9 Compare December 3, 2023 19:58
@illwieckz
Copy link
Member Author

MinGW is now detected too:

-- Detected architecture: amd64
-- CMake generator: Unix Makefiles
-- Detected C compiler: MinGW64 12-posix/gcc-12.0.0 
-- Detected C++ compiler: MinGW64 12-posix/gcc-12.0.0 

@illwieckz illwieckz force-pushed the illwieckz/compiler-string/sync branch 3 times, most recently from 4c02ffd to d232878 Compare December 4, 2023 03:41
@slipher
Copy link
Member

slipher commented Dec 4, 2023

I don't find it worth the hassle to have hundreds of lines of code attempting to parse the compiler's version output. Let's compare this with the architecture determining tool:

  • CMake's architecture information is garbage: inconsistent and poorly documented. So better arch info is sorely needed. Whereas CMake does an OK job telling us what brand of compiler we're using.
  • The architecture tool uses compiler macros which are a well-documented and stable interface. Version strings are free-form and might be changed without warning as they are not intended as a machine-readable interface.

Instead of this, I propose just doing $CC --version and capturing the first line, without further parsing. I tried this with various compilers (mostly from godbolt.org):

gcc (x86_64-posix-seh-rev0, Built by MinGW-W64 project) 8.1.0
g++.exe (MinGW-W64 x86_64-ucrt-mcf-seh, built by Brecht Sanders) 13.1.0
clang version 3.6.0 (https://chromium.googlesource.com/a/native_client/pnacl-clang.git 96b3da27dcefc9d152e51cf54280989b2206d789) (https://chromium.googlesource.com/a/native_client/pnacl-llvm.git d0089f0b008e03cfd141f05c80e3b628c2df75c1) nacl-version=0beb554650e57834a9fdda91ca9e3352b330b653
Intel(R) oneAPI DPC++/C++ Compiler 2023.2.0 (2023.2.0.20230721)
Microsoft (R) C/C++ Optimizing Compiler Version 19.29.30146 for x86
icc (ICC) 19.0.0.117 20180804
clang version 12.0.1 ([email protected]:ziglang/zig-bootstrap.git 8cc2870e09320a390cafe4e23624e8ed40bd363c)
zapcc clang version 5.0.0 (trunk 295600) (based on LLVM 5.0.0svn) built on Feb 26 2017 19:10:16
nvc++ 23.9-0 64-bit target on x86-64 Linux -tp cascadelake 
mips64-unknown-linux-gnu-g++ (crosstool-NG UNKNOWN) 13.1.0
ecc version 2017-07-16 (http://ellcc.org) based on clang version 5.0.0 (trunk 308147)
clang version 12.0.0
i586-pc-msdosdjgpp-g++ (GCC) 6.4.0

Only a couple of them seemed somewhat unsatisfactory: clang-cl which gave the not-terribly-informative clang version 12.0.0; and PNaCl due to its extreme verbosity. But we could always add a special case for the NaCl compiler.

In case it matters that the command returns 0, for MSVC (but NOT for clang-cl which is both MSVC and CLANG) you should just invoke it with no arguments. (With --version it complains about an unrecognized flag but still prints the version in the first line.)

I guess it would be good to print our target triple (as used for external_deps) somewhere too; this could help fill in the missing information for the uninformative Clang case.

To work around the Zig choking on -mtune problem, you can use our try_c_cxx_flag macro which omits the flag if the compiler errors out when compiling a hello world program.

@illwieckz
Copy link
Member Author

The proposed code parses compiler defines. It only parses things like -v output if the defines are not usable, for example:

  1. zig doesn't set any zig define, we can find some zig word in some version string but that's because it's part of reported repository url used to build the compiler (so a custom build may report another url):
    #define __clang_version__ "16.0.1 (https://github.com/ziglang/zig-bootstrap 710c5d12660235bc4eac103a8c6677c61f0a9ded)"
  2. gcc sets many gcc define but almost all compilers mimic gcc and pretend to be gcc and define all thoes gcc defines…

Otherwise the proposed code relies on macro parsing, it does g++ -E -dM emptyfile.cpp which dumps all the compiler-generated defines to standard out, this works with both gcc and clang derivatives and some compilers mimicking gcc like icc. I didn't went the way of what I did for architecture detection because we need to do more parsing out of defines, but the idea is the same.

Unlike CMake architecture detection which is totally unreliable, CMake compiler detection is actually not bad and considered well tested, since CMake has to write millions of compilation commands. The reason why I want extra detection is that most of the time, CMake only reports the underlying compiler, which is totally good enough for its task.

CMake some extra detection for some clang-based compilers like Intel ICX and armclang but even if it would not detect them and considered them all being clang I guess CMake would be fine, like CMake is already fine with AMD AOCC or Saigo by simply considering it's clang.

But I want to know more, like make the difference between GCC and MinGW, between Clang, PNaCl, Saigo, Wasi, etc. I also wan to know which version of underlying compiler is used (which is useful when a bug comes after a specific clang version for example).

So let's look at the code.

Here is code that gets clang version found by CMake, we will use that as “underlying” version if we detect a clang variant. We should be able to count on it. Well, it would not work if someone publishes a new compiler variant based on a variant, like if someone ports Saigo patches over ICX it would be reported as IntelLLVM and not Clang so this code will not work.

function(detect_custom_clang_version lang file)
	# If CMake already detected the compiler as Clang but we know
	# it's something else.
	if(CUSTOM_${lang}_COMPILER_ID AND CMAKE_${lang}_COMPILER_ID STREQUAL "Clang")
		set(CUSTOM_${lang}_CLANG_VERSION "${CMAKE_${lang}_COMPILER_VERSION}")
		set(CUSTOM_${lang}_CLANG_VERSION "${CUSTOM_${lang}_CLANG_VERSION}" PARENT_SCOPE)
		return()
	endif()

Here is probably what CMake does internally, this is a fallback. This is like doing printf(__clang_version__); but with extra parsing because some compilers like to add some data after the number.

	# Parse “<compiler> -E -dM <source file>”
	execute_process(COMMAND "${CMAKE_${lang}_COMPILER}" ${CUSTOM_${lang}_COMPILER_SUBCOMMAND} -E -dM "${file}"
		OUTPUT_VARIABLE CUSTOM_${lang}_CLANG_OUTPUT
		RESULT_VARIABLE CUSTOM_${lang}_RETURN_CODE
		ERROR_QUIET)

	if (NOT CUSTOM_${lang}_RETURN_CODE) # Success
		if ("${CUSTOM_${lang}_CLANG_OUTPUT}" MATCHES "\#define __clang_version__ ")
			string(REGEX REPLACE ".*#define __clang_version__ \"([^ ]+)[\" ]*.*" "\\1" CUSTOM_${lang}_CLANG_VERSION "${CUSTOM_${lang}_CLANG_OUTPUT}")
			set(CUSTOM_${lang}_CLANG_VERSION "${CUSTOM_${lang}_CLANG_VERSION}" PARENT_SCOPE)
		endif()
	endif()
endfunction()

Here is code that gets GCC version found by CMake, we will use that as “underlying” version if we detect a GCC variant. We should be able to count on it.

function(detect_custom_gcc_version lang file)
	# If CMake already detected the compiler as GCC but we know
	# it's something else.
	if(CUSTOM_${lang}_COMPILER_ID AND CMAKE_${lang}_COMPILER_ID STREQUAL "GNU")
		set(CUSTOM_${lang}_GCC_VERSION "${CMAKE_${lang}_COMPILER_VERSION}")
		set(CUSTOM_${lang}_GCC_VERSION "${CUSTOM_${lang}_GCC_VERSION}" PARENT_SCOPE)
		return()
	endif()

Here is probably what CMake does internally, this is a fallback. First we parse gcc -v output because there is no define known to be unique to GCC, then we do something like printf("%d.%d.%d", __GNUC__, __GNUC_MINOR__, __GNUC_PATCHLEVEL__);.

	# Almost all compilers on Earth define __GNUC__, __GNUC_MINOR__, and __GNUC_PATCHLEVEL__,
	# So we first have to check it's really a GCC variant.
	# Parse “<compiler> -v”
	execute_process(COMMAND "${CMAKE_${lang}_COMPILER}" -v
		ERROR_VARIABLE CUSTOM_${lang}_GCC_OUTPUT
		RESULT_VARIABLE CUSTOM_${lang}_RETURN_CODE
		OUTPUT_QUIET)

	if (NOT CUSTOM_${lang}_RETURN_CODE) # Success
		# The existence of this string tells us it's a GCC variant.
		# The version in this string is the same as __VERSION__,
		# the version of the GCC variant, not the version of the upstream
		# GCC we are looking for.
		if ("${CUSTOM_${lang}_GCC_OUTPUT}" MATCHES "\ngcc version ")
			# Parse “<compiler> -E -dM <source file>”
			# No subcommand implemented for now, there may be no usage.
			execute_process(COMMAND "${CMAKE_${lang}_COMPILER}" -E -dM "${file}"
			OUTPUT_VARIABLE CUSTOM_${lang}_GCC_OUTPUT
			RESULT_VARIABLE CUSTOM_${lang}_RETURN_CODE
			ERROR_QUIET)

			if (NOT CUSTOM_${lang}_RETURN_CODE) # Success
				string(REGEX REPLACE ".*#define __GNUC__ ([^ \n]+).*" "\\1" CUSTOM_${lang}_GCC_MAJOR "${CUSTOM_${lang}_GCC_OUTPUT}")
				string(REGEX REPLACE ".*#define __GNUC_MINOR__ ([^ \n]+).*" "\\1" CUSTOM_${lang}_GCC_MINOR "${CUSTOM_${lang}_GCC_OUTPUT}")
				string(REGEX REPLACE ".*#define __GNUC_PATCHLEVEL__ ([^ \n]+).*" "\\1" CUSTOM_${lang}_GCC_PATCHLEVEL "${CUSTOM_${lang}_GCC_OUTPUT}")

				set(CUSTOM_${lang}_GCC_VERSION "${CUSTOM_${lang}_GCC_MAJOR}.${CUSTOM_${lang}_GCC_MINOR}.${CUSTOM_${lang}_GCC_PATCHLEVEL}")
				set(CUSTOM_${lang}_GCC_VERSION "${CUSTOM_${lang}_GCC_VERSION}" PARENT_SCOPE)
				return()
			endif()
		endif()
	endif()
endfunction()

Here is the compiler detection, it requests the compiler to dump defines and parses it. This is similar to what we do for arch detection. It's like doing #ifdef __pnacl__, printf(__VERSION__);, etc.

function(detect_custom_compiler_id_version lang file)
	# Parse “<compiler> -E -dM <source file>”
	execute_process(COMMAND "${CMAKE_${lang}_COMPILER}" -E -dM "${file}"
		OUTPUT_VARIABLE CUSTOM_${lang}_COMPILER_OUTPUT
		RESULT_VARIABLE CUSTOM_${lang}_RETURN_CODE
		ERROR_QUIET)

	if (NOT CUSTOM_${lang}_RETURN_CODE) # Success
		# PNaCL
		if ("${CUSTOM_${lang}_COMPILER_OUTPUT}" MATCHES "\#define __pnacl__ 1")
			set(CUSTOM_${lang}_COMPILER_ID "PNaCl" PARENT_SCOPE)

			string(REGEX REPLACE ".*#define __VERSION__ \"([^ ]+).*" "\\1" CUSTOM_${lang}_COMPILER_VERSION "${CUSTOM_${lang}_COMPILER_OUTPUT}")
			set(CUSTOM_${lang}_COMPILER_VERSION "${CUSTOM_${lang}_COMPILER_VERSION}" PARENT_SCOPE)

			return()
		endif()

		# Saigo
		if ("${CUSTOM_${lang}_COMPILER_OUTPUT}" MATCHES "\#define __saigo__ 1")
			set(CUSTOM_${lang}_COMPILER_ID "Saigo" PARENT_SCOPE)

			string(REGEX REPLACE ".*#define __VERSION__ \"Clang ([^ \"]+).*" "\\1" CUSTOM_${lang}_COMPILER_VERSION "${CUSTOM_${lang}_COMPILER_OUTPUT}")
			set(CUSTOM_${lang}_COMPILER_VERSION "${CUSTOM_${lang}_COMPILER_VERSION}" PARENT_SCOPE)

			return()
		endif()

		# AOCC
		if ("${CUSTOM_${lang}_COMPILER_OUTPUT}" MATCHES "CLANG: AOCC")
			set(CUSTOM_${lang}_COMPILER_ID "AOCC" PARENT_SCOPE)

			string(REGEX REPLACE ".*CLANG: AOCC_([^ \"]+).*" "\\1" CUSTOM_${lang}_COMPILER_VERSION "${CUSTOM_${lang}_COMPILER_OUTPUT}")
			set(CUSTOM_${lang}_COMPILER_VERSION "${CUSTOM_${lang}_COMPILER_VERSION}" PARENT_SCOPE)

			return()
		endif()

		# MinGW64
		# This must be tested before MinGW32 since MinGW64 also defines "__MINGW32__".
		if ("${CUSTOM_${lang}_COMPILER_OUTPUT}" MATCHES "\#define __MINGW64__ 1")
			set(CUSTOM_${lang}_COMPILER_ID "MinGW64" PARENT_SCOPE)

			string(REGEX REPLACE ".*#define __VERSION__ \"([^ \"]+).*" "\\1" CUSTOM_${lang}_COMPILER_VERSION "${CUSTOM_${lang}_COMPILER_OUTPUT}")
			set(CUSTOM_${lang}_COMPILER_VERSION "${CUSTOM_${lang}_COMPILER_VERSION}" PARENT_SCOPE)

			return()
		endif()

		# MinGW32
		if ("${CUSTOM_${lang}_COMPILER_OUTPUT}" MATCHES "\#define __MINGW32__ 1")
			set(CUSTOM_${lang}_COMPILER_ID "MinGW32" PARENT_SCOPE)

			string(REGEX REPLACE ".*#define __VERSION__ \"([^ \"]+).*" "\\1" CUSTOM_${lang}_COMPILER_VERSION "${CUSTOM_${lang}_COMPILER_OUTPUT}")
			set(CUSTOM_${lang}_COMPILER_VERSION "${CUSTOM_${lang}_COMPILER_VERSION}" PARENT_SCOPE)

			return()
		endif()
	endif()

	# Parse “<compiler> --help”
	# There is a bug in cmake: if we set CMAKE_C_COMPILER to "zig:cc",
	# only "zig" is returned when using the ${CMAKE_C_COMPILER} variable,
	# so here the command will be "zig --help".
	execute_process(COMMAND "${CMAKE_${lang}_COMPILER}" --help
		OUTPUT_VARIABLE CUSTOM_${lang}_COMPILER_OUTPUT
		RESULT_VARIABLE CUSTOM_${lang}_RETURN_CODE
		ERROR_QUIET)

	if (NOT CUSTOM_${lang}_RETURN_CODE) # Success
		# Zig
		if ("${CUSTOM_${lang}_COMPILER_OUTPUT}" MATCHES ": zig ")
			set(CUSTOM_${lang}_COMPILER_ID "Zig" PARENT_SCOPE)

			execute_process(COMMAND "${CMAKE_${lang}_COMPILER}" version OUTPUT_VARIABLE CUSTOM_${lang}_COMPILER_VERSION)
			string(STRIP "${CUSTOM_${lang}_COMPILER_VERSION}" CUSTOM_${lang}_COMPILER_VERSION)
			set(CUSTOM_${lang}_COMPILER_VERSION "${CUSTOM_${lang}_COMPILER_VERSION}" PARENT_SCOPE)

			# There is a bug in CMake: if we set CMAKE_C_COMPILER to "zig;cc",
			# only "zig" is returned when using the ${CMAKE_C_COMPILER} variable.
			set(C_SUBCOMMAND "cc")
			set(CXX_SUBCOMMAND "c++")

			set(CUSTOM_${lang}_COMPILER_SUBCOMMAND "${${lang}_SUBCOMMAND}")
			set(CUSTOM_${lang}_COMPILER_SUBCOMMAND "${CUSTOM_${lang}_COMPILER_SUBCOMMAND}" PARENT_SCOPE)

			return()
		endif()
	endif()
endfunction()

Then a code running those functions with both C and C++ compilers:

function(detect_custom_compiler lang)
	set(C_EXT ".c")
	set(CXX_EXT ".cpp")

	string(RANDOM RANDOM_SUFFIX)
	set(COMPILER_TEST_FILE "${PROJECT_BINARY_DIR}/test_compiler_${RANDOM_SUFFIX}${${lang}_EXT}")
	file(TOUCH "${COMPILER_TEST_FILE}")

	detect_custom_compiler_id_version("${lang}" "${COMPILER_TEST_FILE}")
	detect_custom_clang_version("${lang}" "${COMPILER_TEST_FILE}")
	detect_custom_gcc_version("${lang}" "${COMPILER_TEST_FILE}")

	file(REMOVE "${COMPILER_TEST_FILE}")

	set(CUSTOM_${lang}_COMPILER_ID "${CUSTOM_${lang}_COMPILER_ID}" PARENT_SCOPE)
	set(CUSTOM_${lang}_COMPILER_VERSION "${CUSTOM_${lang}_COMPILER_VERSION}" PARENT_SCOPE)
	set(CUSTOM_${lang}_COMPILER_SUBCOMMAND "${CUSTOM_${lang}_COMPILER_SUBCOMMAND}" PARENT_SCOPE)

	set(CUSTOM_${lang}_CLANG_VERSION "${CUSTOM_${lang}_CLANG_VERSION}" PARENT_SCOPE)
	set(CUSTOM_${lang}_GCC_VERSION "${CUSTOM_${lang}_GCC_VERSION}" PARENT_SCOPE)
endfunction()

@illwieckz
Copy link
Member Author

illwieckz commented Dec 4, 2023

I tried to implement it a bit like I did for architectures:

#define STRING(s) #s
#define XSTRING(s) STRING(s)

#define REPORT(key, value) \
	"##REPORT## ##[## DAEMON_" key " ##|## " value " ##]##"
#define REPORT_VERSION_3(name, major, minor, patch) \
	REPORT(name "_VERSION", XSTRING(major) "." XSTRING(minor) "." XSTRING(patch))
#define REPORT_VERSION_2(name, major, minor) \
	REPORT(name "_VERSION", XSTRING(major) "." XSTRING(minor))
#define REPORT_VERSION_1(name, major) \
	REPORT(name "_VERSION", XSTRING(major))
#define REPORT_VERSION_STRING(name, value) \
	REPORT(name "_VERSION_STRING", value)
#define REPORT_COMPATIBILITY(name) \
	REPORT(name "_COMPATIBILITY", "YES")
#define REPORT_NAME(name) \
	REPORT("COMPILER_NAME", name)

// GCC

#if defined(__GNUC__)
	#pragma message(REPORT_COMPATIBILITY("GCC"))
#endif

#if defined(__GNUC__) && defined(__GNUC_MINOR__) && defined(__GNUC_PATCHLEVEL__)
	#pragma message(REPORT_VERSION_3("GCC", __GNUC__, __GNUC_MINOR__, __GNUC_PATCHLEVEL__))
#endif

// Clang

#if defined(__clang__)
	#pragma message(REPORT_COMPATIBILITY("CLANG"))
#endif

#if defined(__clang_major__) && defined(__clang_minor__) && defined(__clang_patchlevel__)
	#pragma message(REPORT_VERSION_3("CLANG", __clang_major__, __clang_minor__, __clang_patchlevel__))
#endif

#if defined(__clang_version__)
	#pragma message(REPORT_VERSION_STRING("CLANG", __clang_version__))
#endif

// ICC

#if defined(__INTEL_COMPILER) && defined(__INTEL_COMPILER_UPDATE)
	#pragma message(REPORT_VERSION_2("ICC", __INTEL_COMPILER, __INTEL_COMPILER_UPDATE))
#elif defined(_ICC)
	#pragma message(REPORT_VERSION_1("ICC", __ICC))
#endif

// ICX

#if defined(__INTEL_CLANG_COMPILER)
	// This would require extra parsing since it's the form 20240000 for 2024.0.0
	#pragma message(REPORT_VERSION("ICX", __INTEL_CLANG_COMPILER)
#elif defined(__INTEL_LLVM_COMPILER)
	// This would require extra parsing since it's the form 20240000 for 2024.0.0
	#pragma message(REPORT_VERSION("ICX", __INTEL_LLVM_COMPILER)
#endif

// Generic

#if defined(__VERSION__)
	#pragma message(REPORT_VERSION_STRING("COMPILER", __VERSION__))
#endif

// Selection

// There is no Zig specific define.

// There is no AOCC specific define, we should parse other defines:
// #define __VERSION__ "AMD Clang 14.0.6 (CLANG: AOCC_4.0.0-Build#434 2022_10_28)"
// #define __clang_version__ "14.0.6 (CLANG: AOCC_4.0.0-Build#434 2022_10_28)"

#if defined(__INTEL_COMPILER) || defined(__ICC)
	#pragma message(REPORT_NAME("ICC")) // Intel
#elif defined(__INTEL_CLANG_COMPILER) || defined(__INTEL_LLVM_COMPILER)
	#pragma message(REPORT_NAME("ICX")) // IntelLLVM
#elif defined(__wasi__)
	#pragma message(REPORT_NAME("WASI"))
#elif defined(__saigo__)
	#pragma message(REPORT_NAME("Saigo"))
#elif defined(__pnacl__)
	#pragma message(REPORT_NAME("PNaCl"))
#elif defined(__MINGW64__) || defined(__MINGW32__)
	#pragma message(REPORT_NAME("MingGW"))
#elif defined(__clang__)
	#pragma message(REPORT_NAME("Clang"))
#elif defined(__GNUC__)
	#pragma message(REPORT_NAME("GCC")) // GNU
#else
	#pragma message(REPORT_NAME("Unknown"))
#endif

This mostly works:

$ icpc -o /dev/null detect.cpp 2>&1 | grep '##REPORT##' | sed -e 's/.*##REPORT## ##\[## //;s/ ##|## /: /;s/ ##]##.*//'
DAEMON_GCC_COMPATIBILITY: YES
DAEMON_GCC_VERSION: 13.2.0
DAEMON_ICC_VERSION: 2021.10
DAEMON_COMPILER_VERSION_STRING: Intel(R) C++ g++ 13.2 mode
DAEMON_COMPILER_NAME: ICC
$ /opt/intel/oneapi/compiler/latest/bin/icpx -o /dev/null detect.cpp 2>&1 | grep '##REPORT##' | sed -e 's/.*##REPORT## ##\[## //;s/ ##|## /: /;s/ ##]##.*//'
DAEMON_GCC_COMPATIBILITY: YES
DAEMON_GCC_VERSION: 4.2.1
DAEMON_CLANG_COMPATIBILITY: YES
DAEMON_CLANG_VERSION: 17.0.0
DAEMON_CLANG_VERSION_STRING: 17.0.0 (icx 2024.0.0.20231017)
DAEMON_COMPILER_VERSION_STRING: Intel(R) oneAPI DPC++/C++ Compiler 2024.0.0 (2024.0.0.20231017)
DAEMON_COMPILER_NAME: ICX
$ wasi-sdk/bin/clang++ -o /dev/null detect.cpp 2>&1 | grep '##REPORT##' | sed -e 's/.*##REPORT## ##\[## //;s/ ##|## /: /;s/ ##]##.*//'
DAEMON_GCC_COMPATIBILITY: YES
DAEMON_GCC_VERSION: 4.2.1
DAEMON_CLANG_COMPATIBILITY: YES
DAEMON_CLANG_VERSION: 16.0.0
DAEMON_CLANG_VERSION_STRING: 16.0.0 
DAEMON_COMPILER_VERSION_STRING: Clang 16.0.0
DAEMON_COMPILER_NAME: WASI

But for some reasons __pnacl__ and __saigo__ are no set:

$ saigo_newlib/bin/clang++ -o /dev/null detect.cpp 2>&1 | grep '##REPORT##' | sed -e 's/.*##REPORT## ##\[## //;s/ ##|## /: /;s/ ##]##.*//'
DAEMON_GCC_COMPATIBILITY: YES
DAEMON_GCC_VERSION: 4.2.1
DAEMON_CLANG_COMPATIBILITY: YES
DAEMON_CLANG_VERSION: 18.0.0
DAEMON_CLANG_VERSION_STRING: 18.0.0 (https://chromium.googlesource.com/a/native_client/nacl-llvm-project-v10.git 65262d842b48c9888e9e2f7eedd1ee3e2ee1b0a7)
DAEMON_COMPILER_VERSION_STRING: Clang 18.0.0 (https://chromium.googlesource.com/a/native_client/nacl-llvm-project-v10.git 65262d842b48c9888e9e2f7eedd1ee3e2ee1b0a7)
DAEMON_COMPILER_NAME: Clang
$ pnacl/bin/clang++ -o /dev/null detect.cpp 2>&1 | grep '##REPORT##' | sed -e 's/.*##REPORT## ##\[## //;s/ ##|## /: /;s/ ##]##.*//'
DAEMON_GCC_COMPATIBILITY: YES
DAEMON_GCC_VERSION: 4.2.1
DAEMON_CLANG_COMPATIBILITY: YES
DAEMON_CLANG_VERSION: 3.6.0
DAEMON_CLANG_VERSION_STRING: 3.6.0 (https://chromium.googlesource.com/a/native_client/pnacl-clang.git 96b3da27dcefc9d152e51cf54280989b2206d789) (https://chromium.googlesource.com/a/native_client/pnacl-llvm.git d0089f0b008e03cfd141f05c80e3b628c2df75c1)
DAEMON_COMPILER_VERSION_STRING: 4.2.1 Compatible Clang 3.6.0 (https://chromium.googlesource.com/a/native_client/pnacl-clang.git 96b3da27dcefc9d152e51cf54280989b2206d789) (https://chromium.googlesource.com/a/native_client/pnacl-llvm.git d0089f0b008e03cfd141f05c80e3b628c2df75c1)
DAEMON_COMPILER_NAME: Clang

I don't know way, I now fail to get __saigo__, __pnacl__, etc. even with <compiler> -E -dM <emptyfile>. 🤷‍♀️️

@illwieckz
Copy link
Member Author

Ah no I feel stupid, I need to use the target compilers, like pnacl-clang++ or x86_64-nacl-clang.

@slipher
Copy link
Member

slipher commented Dec 4, 2023

But I want to know more, like make the difference between GCC and MinGW, between Clang, PNaCl, Saigo, Wasi, etc.

CMake already knows about MinGW. We have various if (MINGW) in our build. As for NaCl compilers, those can only be used via our custom toolchain file so we should just set needed info in the toolchain file.

I also wan to know which version of underlying compiler is used (which is useful when a bug comes after a specific clang version for example).

Making build-time decisions based on a version could have some utility, but not enough to be worth it. If you just want to know what version something was built with you can save the version string without parsing it.

@illwieckz
Copy link
Member Author

The underlying version is useful, not for build-time decision (I simply concatenate it so it's not usable by CMake), but for bisecting bug that only occurs with specific compiler versions like:

In this issue I used such compiler detection to make sure the bug started to appear when a compiler was based on clang > 13.

@illwieckz
Copy link
Member Author

The primary purpose of such detection is to print compiler versions in daemon.log for when we get bug reports.

@illwieckz
Copy link
Member Author

One thing that can be used for decision is like “if gcc-based or clang-based (but not pnacl), enable PCH”.

@illwieckz
Copy link
Member Author

I pushed a different implementation, similar to the one I did to detect architectures (i.e. relying on preprocesor to execute some #ifdef, etc.).

@illwieckz illwieckz force-pushed the illwieckz/compiler-string/sync branch from 6271bad to d010aff Compare December 5, 2023 05:04
@illwieckz
Copy link
Member Author

Note that DaemonCompiler.c and DaemonCompiler.cpp are the exact same file, CMake is annoying because it requires the file extension, I'll look for a solution without file duplication later.

@illwieckz illwieckz force-pushed the illwieckz/compiler-string/sync branch 2 times, most recently from 8a469d3 to b814da8 Compare September 12, 2024 01:33
@illwieckz illwieckz force-pushed the illwieckz/compiler-string/sync branch from b814da8 to 2044eca Compare November 4, 2024 15:53
@illwieckz illwieckz force-pushed the illwieckz/compiler-string/sync branch 5 times, most recently from 5684640 to 698436a Compare December 3, 2024 05:10
@illwieckz
Copy link
Member Author

At the risk of throwing additional fuel on the fire...

We could use https://github.com/sbellus/json-cmake to output JSON so the parsing could become much simpler....

This can't help, all we can do is some compiler's pragma message.

We can't produce json with compiler's pragma message.

What the compiler outputs looks like this:

cmake/DaemonCompiler/DaemonCompiler.c:52:10: warning: <REPORT<DAEMON_COMPILER_GCC_COMPATIBILITY=ON>REPORT> [-W#pragma-messages]
   52 |         #pragma message(REPORT_COMPATIBILITY("GCC"))
      |                 ^
cmake/DaemonCompiler/DaemonCompiler.c:56:10: warning: <REPORT<DAEMON_COMPILER_GCC_VERSION=4.2.1>REPORT> [-W#pragma-messages]
   56 |         #pragma message(REPORT_VERSION_3("GCC", __GNUC__, __GNUC_MINOR__, __GNUC_PATCHLEVEL__))
      |                 ^
cmake/DaemonCompiler/DaemonCompiler.c:62:10: warning: <REPORT<DAEMON_COMPILER_Clang_COMPATIBILITY=ON>REPORT> [-W#pragma-messages]
   62 |         #pragma message(REPORT_COMPATIBILITY("Clang"))
      |                 ^
cmake/DaemonCompiler/DaemonCompiler.c:66:10: warning: <REPORT<DAEMON_COMPILER_Clang_VERSION=19.0.0>REPORT> [-W#pragma-messages]
   66 |         #pragma message(REPORT_VERSION_3("Clang", __clang_major__, __clang_minor__, __clang_patchlevel__))
      |                 ^
cmake/DaemonCompiler/DaemonCompiler.c:70:10: warning: <REPORT<DAEMON_COMPILER_Clang_VERSION_STRING=19.0.0git (https://chromium.googlesource.com/a/native_client/nacl-llvm-project-v10.git e25355fddbdece2ef08747ead05b7f69f3bc6dca)>REPORT> [-W#pragma-messages]
   70 |         #pragma message(REPORT_VERSION_STRING("Clang", __clang_version__))
      |                 ^
cmake/DaemonCompiler/DaemonCompiler.c:109:10: warning: <REPORT<DAEMON_COMPILER_generic_VERSION_STRING=Clang 19.0.0git (https://chromium.googlesource.com/a/native_client/nacl-llvm-project-v10.git e25355fddbdece2ef08747ead05b7f69f3bc6dca)>REPORT> [-W#pragma-messages]
  109 |         #pragma message(REPORT_VERSION_STRING("generic", __VERSION__))
      |                 ^
cmake/DaemonCompiler/DaemonCompiler.c:139:10: warning: <REPORT<DAEMON_COMPILER_NAME=Saigo>REPORT> [-W#pragma-messages]
  139 |         #pragma message(REPORT_NAME("Saigo"))
      |                 ^
7 warnings generated.

@illwieckz
Copy link
Member Author

Someone claimed that the results you get for this depend on the version you put in cmake_minimum_required and will be according to the documentation for that version.

I never seen it, and even if it was true it would be a motivation to do it ourselves instead.

No. My suggestion was to use the --version string for making the string to display at runtime. if (MINGW) is used for making build-time decisions and works fine for that.

I'm not against adding the output of --version to some define but that would be to be as an extra to this, not as a replacement to this. Also to properly call --version one may have to know the compiler first. For example ICC now requires to do --diag-disable=10441 --version otherwise the first line is a warning that is printed before the version line. I don't know what is the command line for MSVC but I would not be surprised if the option is not --version. For zig, what do we want, zig cc --version (reporting clang version) or zig version (reporting zig version)? This code actually reports both.

Also just capturing the --version output doesn't help when custom configuration has to be done, the MINGW may have been a bad example, but for example ICC doesn't support -march=x86-64 (Maybe Intel did not liked that name that was made-up by AMD…) and Zig doesn't support -mtune.

An alternative to also print the --version output would be to also print things like __VERSION__ string or the __clang_version__ one, here is the comparison of them on Saigo:

  • --version:
    clang version 19.0.0git (https://chromium.googlesource.com/a/native_client/nacl-llvm-project-v10.git e25355fddbdece2ef08747ead05b7f69f3bc6dca)
  • __clang_version__:
    19.0.0git (https://chromium.googlesource.com/a/native_client/nacl-llvm-project-v10.git e25355fddbdece2ef08747ead05b7f69f3bc6dca)

This can be useful because of the commit refs my code strips out, but what I output is much more readable.

This PR now brings much more than just generating some macros to report the compiler used to build the game, it also brings more compiler support thanks to the detection, and also improves things like extending PCH support to compilers with sub-commands.

Here is some example of nexe cgame reporting which compiler was used to build it:

  • cgame NaCl nacl (PNaCl 4.2.1/clang-3.6.0 pnacl-clang++) Dec 3 2024
  • cgame NaCl nacl (Saigo 19.0.0/clang-19.0.0 x86_64-nacl-clang) Dec 3 2024

Such report is also useful for compiled games as Saigo suffered from the model bug but PNaCl didn't.

Regarding Zig, I have a long term side-project to experiment cross-compiling native binaries with it, that's why I'm doing some efforts to improve support of it and part of this compiler detection contributes to that.

@slipher
Copy link
Member

slipher commented Dec 3, 2024

Still don't believe it's worth adding +500 lines of code just to print a compiler version string. Not worth the increase in complexity.

I don't know/remember what the motivation for this PR is, but if you're worried about forgetting which compiler a release was built with, there's this: https://unix.stackexchange.com/a/722. Clang does it too.

@illwieckz
Copy link
Member Author

illwieckz commented Dec 3, 2024

My first purpose is to have the information in daemon.log.

Let's take the model bug as an example: someone reports distorted models when he builds the game on his side, we ask for the daemon.log file, the compiler information can be used as a guess along other things like OpenGL driver version, etc.

The same happens when some people report an engine bug when running the game from AUR on Arch, we can ask for the daemon.log file, the compiler information can be used as a guess along other things like OpenGL driver version, etc.

First motivation is to report in daemon.log the compiler version used to build the C++ code, the same way we report the GLSL compiler version.

Then the second motivation is to use that information to improve compiler support for various reasons.

What motivated me at first was the console offset on clang:

The model bug is another example where reporting the compiler can be useful:

The more people will build the game themselves, or the more Linux distributions may package the game for their users, the more this kind of bug will be expected to be reported in the future, and from people having less knowledge than the core development team. And since we want to reach a wider audience, it means we want a situation where bugs specific to compilers are expected to be reported more from people with less debugging skills.

@illwieckz illwieckz force-pushed the illwieckz/compiler-string/sync branch 5 times, most recently from e906b7f to 6ebbd9f Compare December 7, 2024 07:48
@illwieckz
Copy link
Member Author

As an example with latest ICX:

/opt/intel/oneapi/compiler/latest/bin/icpx -v
Intel(R) oneAPI DPC++/C++ Compiler 2025.0.4 (2025.0.4.20241205)

This doesn't report the compiler use Clang and which version of Clang it uses.

But this does:

echo '' | /opt/intel/oneapi/compiler/latest/bin/icpx -E -dM - | grep __clang_version__
#define __clang_version__ "19.0.0git (icx 2025.0.4.20241205)"

@illwieckz illwieckz force-pushed the illwieckz/compiler-string/sync branch from 6ebbd9f to 4313b20 Compare January 7, 2025 00:49
@illwieckz
Copy link
Member Author

With the extra logging, this PR also brings interesting features to CMake: for example knowing the underlying compiler gives the ability to use Clang-specific compile flags by knowing the compiler is Clang-based even if CMake itself reports something else, and other things like that become possible.

So, the only opinion against is that it adds code. But I want those features and this is the best implementation I can do. The proposed alternatives don't match the need. Actually the code si ready for more than a year and it works. After a year, the need isn't gone, and I have plan to improve the logging in other ways and to avoid merge conflicts I blocked those other changes for the same time.

So I will simply merge this once the CI finishes.

@illwieckz illwieckz merged commit 4313b20 into master Jan 7, 2025
9 checks passed
@illwieckz illwieckz deleted the illwieckz/compiler-string/sync branch January 7, 2025 01:14
slipher added a commit to slipher/Daemon that referenced this pull request Jan 12, 2025
Revert "framework: report which compiler was used to build the game"

This reverts commit 4313b20.

Revert "cmake: make possible to use PCH with compilers with subcommands"

This reverts commit 098edeb.

Revert "cmake: reverse the PCH PNaCl test"

This reverts commit 1b51cc3.

Revert "cmake: improve the PCH support detection"

This reverts commit e552ff7.

Revert "cmake: add comments for future self to improve the CMake code"

This reverts commit 4cb34f5.

Revert "cmake: improve compiler support"

This reverts commit cbe119e.

Revert "cmake: report wich compiler is used to build the game"

This reverts commit 596020e.
@illwieckz
Copy link
Member Author

@slipher I answer here because I don't want to pollute that other thread with that topic:

This is a good example of why we should not merge a complex PR like #982 without any code review.

It was reviewed by you.

For example you said there:

I don't find it worth the hassle to have hundreds of lines of code attempting to parse the compiler's version output. Let's compare this with the architecture determining tool:

  • The architecture tool uses compiler macros which are a well-documented and stable interface. Version strings are free-form and might be changed without warning as they are not intended as a machine-readable interface.

So, based on that review of you, I entirely rewrote that compiler detection to be written the way of the architecture determining tool. In fact it was already using compiler macros, but instead of parsing the output of the cc -dM -E (which is intended as a machine-readable interface), I went the warning-emitting-and-parsing way like we already did for the architecture detection, as you said. In fact this made the parsing more convoluted but I didn't mind.

This PR has been reviewed by you more than a year ago and I rewrote accordingly.

So, after more than a year of review by you, once I had addressed all your remarks about the implementation, once you had nothing to review anymore about the implementation, all that was remaining was your opinion (here) on the idea of having this feature or not.

Still don't believe it's worth adding +500 lines of code just to print a compiler version string.

Actually you may haven't grasped yet the purposes of that feature as every time you describe what you think is its purpose, you're close but not right. It's like saying “you want bread” instead of “you're hungry”, it's close but not right.

I believe we need that feature so any code providing that is worth to add. We don't share this opinion, but the review was done, all that was remaining were opinions and I was in position to chose among the opinions to follow.

I also believe it's not useful to discuss more the need of this. So hiding reverts under pull requests titles named “Fix” or other misleading move is not welcome, because it would not be welcome even if it was named “Revert” explicitly to begin with.

@slipher
Copy link
Member

slipher commented Jan 21, 2025

This is a good example of why we should not merge a complex PR like #982 without any code review.

It was reviewed by you.

Only to say that I disapprove of the general idea. I didn't go over the code to look for mistakes improvements.

For example you said there:

I don't find it worth the hassle to have hundreds of lines of code attempting to parse the compiler's version output. Let's compare this with the architecture determining tool:

  • The architecture tool uses compiler macros which are a well-documented and stable interface. Version strings are free-form and might be changed without warning as they are not intended as a machine-readable interface.

So, based on that review of you, I entirely rewrote that compiler detection to be written the way of the architecture determining tool. In fact it was already using compiler macros, but instead of parsing the output of the cc -dM -E (which is intended as a machine-readable interface), I went the warning-emitting-and-parsing way like we already did for the architecture detection, as you said.

To say that I suggested this or that rewrite is twisting my words far from their intended meaning. In that case it sounds like my comment was inaccurate or out of date anyway.

So, after more than a year of review by you, once I had addressed all your remarks about the implementation, once you had nothing to review anymore about the implementation, all that was remaining was your opinion (here) on the idea of having this feature or not.

I never got to the point of commenting on the implementation details. If I dislike the idea of a PR but there is a majority of devs in favor, I may still comment on the implementation to suggest improvements. Since excluding its author, zero developers expressed support of the PR, I did not proceed to comment on the implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants