Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

For future reference: static linking across different platforms #6741

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

Nielsbishere
Copy link
Contributor

@Nielsbishere Nielsbishere commented Jul 1, 2024

The following PR is meant more as a showcase of hacky solutions to get static linking to work across different platforms.
I'm not expecting this to get merged of course, it's just to demonstrate my findings and hopefully something can be improved internally to fix these issues in a better way down the road.
It demonstrates the following issues, that when solved can make static linking possible:

  • DLL hooks should be made into DxcInitialize and DxcShutdown which the user needs to manually call if static linking is applied.
  • DXC_API_IMPORT becomes a NO-OP in static linking.
  • LLVMHLSL, LLVMipo and LLVMScalarOpts have some weird circular dependency that makes linking impossible on non MSVC it seems like. In this branch, I merged them all in one, which is a terrible solution, but it works. Ideally, the circular dependency should be resolved instead. clangAnalysis and clangAST have the same problem, so I also merged those.
  • DXC CLI tools expect dynamic linking, so I turned them off (I only needed dxcompiler.lib/.a, etc.).

As an additional note, I hope the wchars can be phased out slowly too, as on linux/OSX these unexpectedly (for me at least) these are UTF32 rather than UTF16. UTF8 would be nice.

Merge back new DXC version's main
…linked but only to be used as a dependency rather than CLI. To keep existing functionality, this define has to be set before including dxcapi.h and the library calling has to call DxcInitialize at the start of the program (or at least before calling any DXC function) and DxcShutdown when DXC should be cleaned up correctly. When enabling static linking, the CLI projects are disabled since they assume dynamic linking. DllMain has been modified to deal with this properly as well.
…ese commits need to be cleaned up at some point because it's very hacky, but it seems to build & link on Linux (execution isn't tested yet). Proper fix is fixing the dependencies, but ain't nobody got time for that.
… prevent recursive issues that's present on linux builds (but only on GitHub CI runners?)
@Nielsbishere Nielsbishere requested a review from a team as a code owner July 1, 2024 21:56
Copy link
Contributor

github-actions bot commented Jul 1, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 1fefbc429bad6621e5de3a97722df0f6d95a14c5 3ff5117c6479b4f9e0b01dab1da5f6e29dcdb365 -- include/dxc/dxcapi.h tools/clang/tools/dxcompiler/DXCompiler.cpp tools/clang/tools/dxcompiler/dxcapi.cpp
View the diff from clang-format here.
diff --git a/include/dxc/dxcapi.h b/include/dxc/dxcapi.h
index b619d3c85..d2ac84228 100644
--- a/include/dxc/dxcapi.h
+++ b/include/dxc/dxcapi.h
@@ -47,14 +47,16 @@ struct IDxcIncludeHandler;
 
 /// \brief Typedef for DxcCreateInstance function pointer.
 ///
-/// This can be used with GetProcAddress to get the DxcCreateInstance function (if it's not statically linked).
+/// This can be used with GetProcAddress to get the DxcCreateInstance function
+/// (if it's not statically linked).
 typedef HRESULT(__stdcall *DxcCreateInstanceProc)(_In_ REFCLSID rclsid,
                                                   _In_ REFIID riid,
                                                   _Out_ LPVOID *ppv);
 
 /// \brief Typedef for DxcCreateInstance2 function pointer.
 ///
-/// This can be used with GetProcAddress to get the DxcCreateInstance2 function (if it's not statically linked).
+/// This can be used with GetProcAddress to get the DxcCreateInstance2 function
+/// (if it's not statically linked).
 typedef HRESULT(__stdcall *DxcCreateInstance2Proc)(_In_ IMalloc *pMalloc,
                                                    _In_ REFCLSID rclsid,
                                                    _In_ REFIID riid,
@@ -90,12 +92,14 @@ extern "C" DXC_API_IMPORT
 
 #ifdef ENABLE_DXC_STATIC_LINKING
 
-/// \brief This function has to be called before any DXC calls to initialize it if it's statically linked.
+/// \brief This function has to be called before any DXC calls to initialize it
+/// if it's statically linked.
 ///
 /// If dynamically linked, this will automatically be called.
 extern "C" HRESULT __stdcall DxcInitialize();
 
-/// \brief This function has to be called after all DXC calls (shutdown) to free everything it if it's statically linked.
+/// \brief This function has to be called after all DXC calls (shutdown) to free
+/// everything it if it's statically linked.
 ///
 /// If dynamically linked, this will automatically be called.
 extern "C" void __stdcall DxcShutdown(BOOL isProcessTermination);
diff --git a/tools/clang/tools/dxcompiler/DXCompiler.cpp b/tools/clang/tools/dxcompiler/DXCompiler.cpp
index 21ecf584a..e12ab0e69 100644
--- a/tools/clang/tools/dxcompiler/DXCompiler.cpp
+++ b/tools/clang/tools/dxcompiler/DXCompiler.cpp
@@ -102,7 +102,7 @@ HRESULT __stdcall DxcInitialize() {
 void __stdcall DxcShutdown(BOOL isProcessTermination) {
 
 #if defined(LLVM_ON_UNIX)
-  (void) isProcessTermination;
+  (void)isProcessTermination;
   DxcSetThreadMallocToDefault();
   ::hlsl::options::cleanupHlslOptTable();
   ::llvm::sys::fs::CleanupPerThreadFileSystem();
@@ -115,7 +115,8 @@ void __stdcall DxcShutdown(BOOL isProcessTermination) {
   ::hlsl::options::cleanupHlslOptTable();
   ::llvm::sys::fs::CleanupPerThreadFileSystem();
   ::llvm::llvm_shutdown();
-  if (!isProcessTermination) { // FreeLibrary has been called or the DLL load failed
+  if (!isProcessTermination) { // FreeLibrary has been called or the DLL load
+                               // failed
     DxilLibCleanup(DxilLibCleanUpType::UnloadLibrary);
   } else { // Process termination. We should not call FreeLibrary()
     DxilLibCleanup(DxilLibCleanUpType::ProcessTermination);
@@ -130,7 +131,7 @@ void __stdcall DxcShutdown(BOOL isProcessTermination) {
 #ifndef ENABLE_DXC_STATIC_LINKING
 #if defined(LLVM_ON_UNIX)
 HRESULT __attribute__((constructor)) DllMain() { return DxcInitialize(); }
-void __attribute__((destructor)) DllShutdown(){ DxcShutdown();}
+void __attribute__((destructor)) DllShutdown() { DxcShutdown(); }
 #else  // LLVM_ON_UNIX
 BOOL WINAPI DllMain(HINSTANCE hinstDLL, DWORD Reason, LPVOID reserved) {
   BOOL result = TRUE;
  • Check this box to apply formatting changes to this branch.

…ble to change the path that pointed to optimizer.hpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: New
Development

Successfully merging this pull request may close these issues.

1 participant