build: Add ClangCL profiles #791

alichraghi · 2024-11-21T14:13:21Z

Also fix some compilation errors catched by clang

Description

Testing

TODO list:

Also fix some compilation errors catched by clang Signed-off-by: Ali Cheraghi <[email protected]>

3rdparty/CMakeLists.txt

3rdparty/dxc/CMakeLists.txt

CMakeLists.txt

cmake/adjust/template/vendor/CXX_MSVC.cmake

cmake/common.cmake

src/nbl/builtin/utils.cmake

CMakeLists.txt

Signed-off-by: Ali Cheraghi <[email protected]>

cmake/adjust/flags.cmake

Signed-off-by: Ali Cheraghi <[email protected]>

include/nbl/builtin/hlsl/cpp_compat/impl/intrinsics_impl.hlsl

devshgraphicsprogramming · 2025-04-08T14:15:26Z

3rdparty/CMakeLists.txt

-		else()
-			set_property(TARGET ${trgt} PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
-		endif()
+		set_property(TARGET ${trgt} PROPERTY MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>$<$<BOOL:${NBL_COMPILER_DYNAMIC_RUNTIME}>:DLL>")


AFAIK in Clang and GCC there are options about static linking of libstd-c++

yes there are and this property has effect only if MSVC ABI is the target

devshgraphicsprogramming · 2025-04-08T14:16:35Z

3rdparty/dxc/CMakeLists.txt

-else()
-	list(APPEND NBL_DXC_CMAKE_OPTIONS "-DCMAKE_MSVC_RUNTIME_LIBRARY:STATIC=MultiThreaded$<$<CONFIG:Debug>:Debug>")		
-endif()
+list(APPEND NBL_DXC_CMAKE_OPTIONS "-DCMAKE_MSVC_RUNTIME_LIBRARY:STATIC=MultiThreaded$<$<CONFIG:Debug>:Debug>$<$<BOOL:${NBL_COMPILER_DYNAMIC_RUNTIME}>:DLL>")


same as with the general nabla comment

#791 (comment)

devshgraphicsprogramming · 2025-04-08T14:17:45Z

CMakeLists.txt

+	if(NOT NBL_COMPILER_DYNAMIC_RUNTIME)
+		message(FATAL_ERROR "Turn NBL_COMPILER_DYNAMIC_RUNTIME on! For dynamic Nabla builds dynamic runtime is mandatory!")
 	endif()


aah ok so we want to ban compiling with static-libstdc++

yes we agreed it must match

devshgraphicsprogramming · 2025-04-08T14:19:20Z

cmake/adjust/template/vendor/CXX_Clang.cmake

+list(APPEND NBL_CXX_COMPILE_OPTIONS
+	-Wextra
+	-fno-strict-aliasing
+	-msse4.2


I think you might also need a flag for stripping debug symbols

devshgraphicsprogramming · 2025-04-08T14:19:41Z

cmake/adjust/template/vendor/CXX_Clang.cmake

+	-Wno-sequence-point
+	-Wno-unused-parameter
+	-Wno-unused-but-set-parameter
+	-Wno-c++98-compat
+	-Wno-c++98-compat-pedantic
+	-Wno-padded
+	-Wno-unsafe-buffer-usage
+	-Wno-switch-enum
+	-Wno-error=ignored-attributes
+	-Wno-error=unused-function
+	-Wno-error=unused-variable
+	-Wno-error=unused-parameter
+	-Wno-error=ignored-attributes
+	-Wno-error=non-pod-varargs


I take it that we'll remove this over time?

yes, @YasInvolved is going to kill all warnings

devshgraphicsprogramming · 2025-04-08T14:20:01Z

cmake/adjust/template/vendor/CXX_Clang.cmake

+	-Wno-error=unused-parameter
+	-Wno-error=ignored-attributes
+	-Wno-error=non-pod-varargs
+	-fno-exceptions


I dont think we can compile without exceptions, there are some try-catch block in our code IIRC ?

devshgraphicsprogramming · 2025-04-08T14:20:43Z

cmake/adjust/template/vendor/CXX_Clang.cmake

+set(NBL_CXX_RELEASE_COMPILE_OPTIONS
+	-fexpensive-optimizations
+)


I think O3 enables this by default ?

devshgraphicsprogramming · 2025-04-08T14:21:05Z

cmake/adjust/template/vendor/CXX_Clang.cmake

+# RelWithDebInfo
+set(NBL_CXX_RELWITHDEBINFO_COMPILE_OPTIONS "")


what about O1 or something ? Alos don't you need ggdb3 ?

devshgraphicsprogramming · 2025-04-08T14:21:06Z

cmake/adjust/template/vendor/CXX_Clang.cmake

+
+# Debug
+set(NBL_CXX_DEBUG_COMPILE_OPTIONS
+	-ggdb3 -Wall -fno-omit-frame-pointer -fstack-protector-strong


is ggdb3 sufficient?

shouldn't -g also be used?

btw GCC/Clang have an equivalent of ffast math that we use in MSVC

actually ggdb3 doesn't exist on clang and it throws a warning about unknown arg

yeah I swear it was a GCC / Emscripten thing

also ggdb3 is for GDB, I think on Clang one is supposed to use LLDB

devshgraphicsprogramming · 2025-04-08T14:23:02Z

cmake/adjust/template/vendor/CXX_Clang.cmake

+# our pervious flags-set function called this, does not affect flags nor configs so I will keep it here temporary
+# TODO: move it out from the profile
+link_libraries(-fuse-ld=gold)


what about incremental linking and stuff?

devshgraphicsprogramming · 2025-04-08T14:23:50Z

cmake/adjust/template/vendor/CXX_Clang.cmake

+include_guard(GLOBAL)
+
+# Debug


what about macro definitions like NDEBUG ?

P.S. should MSVC even define should macros from commandline instead of configuring theminto the header?

devshgraphicsprogramming · 2025-04-08T14:24:48Z

cmake/adjust/template/vendor/C_Clang.cmake

+# Debug
+set(NBL_C_DEBUG_COMPILE_OPTIONS
+	-ggdb3 -Wall -fno-omit-frame-pointer -fstack-protector-strong
+)
+
+# Release
+set(NBL_C_RELEASE_COMPILE_OPTIONS
+	-fexpensive-optimizations
+)
+
+# RelWithDebInfo
+set(NBL_C_RELWITHDEBINFO_COMPILE_OPTIONS "")
+
+# Global
+list(APPEND NBL_C_COMPILE_OPTIONS
+	-Wextra
+	-fno-strict-aliasing
+	-msse4.2
+	-maes
+	-mfpmath=sse		
+	-Wextra
+	-Wno-sequence-point
+	-Wno-unused-parameter
+	-Wno-unused-but-set-parameter
+	-Wno-c++98-compat
+	-Wno-c++98-compat-pedantic
+	-Wno-padded
+	-Wno-unsafe-buffer-usage
+	-Wno-switch-enum
+	-Wno-error=ignored-attributes
+	-Wno-error=unused-function
+	-Wno-error=unused-variable
+	-Wno-error=unused-parameter
+	-Wno-error=ignored-attributes
+	-Wno-error=non-pod-varargs
+	-fno-exceptions
+)
+
+if(NBL_SANITIZE_ADDRESS)
+	list(APPEND NBL_C_COMPILE_OPTIONS -fsanitize=address)
+endif()
+
+if(NBL_SANITIZE_THREAD)
+	list(APPEND NBL_C_COMPILE_OPTIONS -fsanitize=thread)
+endif()
+
+# our pervious flags-set function called this, does not affect flags nor configs so I will keep it here temporary
+# TODO: move it out from the profile
+link_libraries(-fuse-ld=gold)


same comments as for C++, maybe unify a bit with a common cmake include ?

devshgraphicsprogramming · 2025-04-08T14:35:34Z

include/nbl/asset/IDescriptorSetLayout.h

@@ -330,7 +330,7 @@ class IDescriptorSetLayout : public IDescriptorSetLayoutBase
 				bindings[i].binding = i;
 				bindings[i].type = type;
 				bindings[i].createFlags = SBinding::E_CREATE_FLAGS::ECF_NONE;
-				bindings[i].stageFlags = stageAccessFlags ? stageAccessFlags[i]:asset::IShader::ESS_ALL_OR_LIBRARY;
+				bindings[i].stageFlags = stageAccessFlags ? stageAccessFlags[i]:asset::IShader::E_SHADER_STAGE::ESS_ALL_OR_LIBRARY;


hlsl::ShaderStage`

devshgraphicsprogramming · 2025-04-08T14:37:03Z

include/nbl/macros.h

 //! Workarounds for compiler specific bugs
 // MSVC 2019 is a special snowflake
-#if defined(_MSC_VER) && _MSC_VER>=1920
+#if defined(_MSC_VER) && !defined(__clang__) && _MSC_VER>=1920


can we just ban old MSC_VER and old GCC ? and not do weird macro tricks ( or less)

devshgraphicsprogramming · 2025-04-08T14:38:52Z

include/nbl/video/ISwapchain.h

+            SSharedCreationParams() {}
+


we'll lose designated initializers with a default ctor

src/nbl/video/IGPUAccelerationStructure.cpp

…er new compile errors after upgrading VS and toolsets (coming from DXC source files)

… NBL_REQUEST_COMPILE_OPTION_SUPPORT, add required instruction set features for simdjson explicitly; now I hit GLI errors due to bad templates

AnastaZIuk · 2025-04-11T13:42:44Z

cmake/adjust/template/vendor/impl/Clang.cmake

+
+	# also instead of enabling single options maybe we could consider requesting an
+	# instruction implementation set instead, eg -march=haswel, though this approach
+	# could add a few more flags then we actually need while building - to rethink


@devshgraphicsprogramming

https://github.com/llvm/llvm-project/blob/b46f16c8d7f4a9e37271cdc4717dd858a4c813dc/llvm/lib/Target/X86/X86.td#L937

… (to latest and our own fork not mine, this one was 6 years old) submodules

build: Add ClangCL profiles

60e1e5b

Also fix some compilation errors catched by clang Signed-off-by: Ali Cheraghi <[email protected]>

alichraghi requested a review from AnastaZIuk November 21, 2024 14:13

AnastaZIuk reviewed Nov 21, 2024

View reviewed changes

alichraghi added 2 commits November 21, 2024 20:21

build: one liner ifs and some fixes

114c549

Signed-off-by: Ali Cheraghi <[email protected]>

update dxc submodule

ff5513b

Signed-off-by: Ali Cheraghi <[email protected]>

AnastaZIuk reviewed Nov 21, 2024

View reviewed changes

cmake/adjust/flags.cmake Outdated Show resolved Hide resolved

alichraghi and others added 2 commits November 22, 2024 20:21

build: simplify if

44acfcf

Signed-off-by: Ali Cheraghi <[email protected]>

Merge branch 'ali_clang' of github.com:Devsh-Graphics-Programming/Nabla

dbff78d

AnastaZIuk force-pushed the ali_clang branch from f44c0eb to dbff78d Compare April 8, 2025 12:38

AnastaZIuk added 2 commits April 8, 2025 14:39

fixing CLang build, save work

616f7d7

make Nabla Clang build work

aad8bb1

AnastaZIuk reviewed Apr 8, 2025

View reviewed changes

include/nbl/builtin/hlsl/cpp_compat/impl/intrinsics_impl.hlsl Outdated Show resolved Hide resolved

bad typo

7b8cb61

devshgraphicsprogramming reviewed Apr 8, 2025

View reviewed changes

src/nbl/video/IGPUAccelerationStructure.cpp Show resolved Hide resolved

update profiles & flags check handle, take care of Clang profile; ent…

062b5ba

…er new compile errors after upgrading VS and toolsets (coming from DXC source files)

update dxc submodule with fixed clang 19.1.1 build, upgrade & correct…

39bb3e1

… NBL_REQUEST_COMPILE_OPTION_SUPPORT, add required instruction set features for simdjson explicitly; now I hit GLI errors due to bad templates

AnastaZIuk reviewed Apr 11, 2025

View reviewed changes

update GLI (use custom location for GLM + fix with templates) and GLM…

cbb4db1

… (to latest and our own fork not mine, this one was 6 years old) submodules

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: Add ClangCL profiles #791

build: Add ClangCL profiles #791

alichraghi commented Nov 21, 2024

devshgraphicsprogramming Apr 8, 2025

AnastaZIuk Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

AnastaZIuk Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

AnastaZIuk Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

AnastaZIuk Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

AnastaZIuk Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

devshgraphicsprogramming Apr 8, 2025

AnastaZIuk Apr 11, 2025

AnastaZIuk Apr 11, 2025

		# RelWithDebInfo
		set(NBL_CXX_RELWITHDEBINFO_COMPILE_OPTIONS "")

		include_guard(GLOBAL)

		# Debug

build: Add ClangCL profiles #791

Are you sure you want to change the base?

build: Add ClangCL profiles #791

Conversation

alichraghi commented Nov 21, 2024

Description

Testing

TODO list:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment