Allow building LLVM 13 with newer compilers#8028
Conversation
| # Allow building with newer compilers | ||
| -DCMAKE_CXX_FLAGS="-include cstdint" |
There was a problem hiding this comment.
Shouldn't we add this line somewhere in the source code rather than in cmake?
There was a problem hiding this comment.
@jkwak-work you mean, in the LLVM source code?
|
It seems like this issue was addressed a while ago Maybe we should use a newer version of LLVM? Claude Code told me the following,● Based on my research, the minimal LLVM version bump needed to fix the uintptr_t/cstdint issue is llvmorg-14.0.5. Here's what I found:The Issue
The Fix
RecommendationUpdate the Slang build script from: This is the closest version to 13.0.1 that includes the GCC 13 compatibility fixes. LLVM 14.0.5 was specifically a bug-fix release that maintained API/ABI compatibility with 14.0.0, This approach is much cleaner than the CMake workaround in PR #8028 and addresses the root cause rather than just patching the symptoms. |
|
@jkwak-work yes, I already linked to llvm/llvm-project@ff1681d in my PR description above. I have no problem with upgrading our LLVM dependency, but I don't know what that would break in Slang, so I didn't try it. |
|
@jkwak-work I see a couple other instances of LLVM 13 mentioned in our codebase:
Would this mean we'd need to update our dependencies on glslang and SPIR-V Tools too? |
|
Also, would updating LLVM constitute a breaking change or a non-breaking change? |
|
I opened #8031 with an attempt to upgrade from LLVM 13 to LLVM 14; seems to work after some minor changes. |
|
Nothing should break for an LLVM update. We use LLVM at a very coarse granularity. |
|
Makes sense. I wasn't sure what LLVM version we actually want to upgrade to, so I just made PRs for all of them, so we can choose whichever we prefer:
|
|
Closing this PR, since we have decided to upgrade to LLVM 14.6 PR which won't have this problem. |
Full set of mutually exclusive choices for upgrading LLVM: - #8031 (you are here) - #8035 - #8036 - #8034 - #8038 - #8039 - #8033 Alternative to #8028. Required some minor changes due to these upstream commits: - llvm/llvm-project@e463b69 - llvm/llvm-project@89b5706
Full set of mutually exclusive choices for upgrading LLVM: - shader-slang#8031 (you are here) - shader-slang#8035 - shader-slang#8036 - shader-slang#8034 - shader-slang#8038 - shader-slang#8039 - shader-slang#8033 Alternative to shader-slang#8028. Required some minor changes due to these upstream commits: - llvm/llvm-project@e463b69 - llvm/llvm-project@89b5706
Currently
external/build-llvm.shworks with GCC 11 which we use in CI, but does not work with GCC 13 or newer:And also does not work with any version of Clang that I've tried, including Clang 12 and Clang 13 (although I don't quite understand why Clang 13 wouldn't be able to build itself):
This is because GCC 13 started requiring explicit
#include <cstdint>, which was not added tollvm/include/llvm/Support/Signals.huntil llvm/llvm-project@ff1681d a few months after thellvmorg-13.0.1tag.This PR allows LLVM 13 to be built with Clang, and with newer versions of GCC, by implicitly including
<cstdint>.