Skip to content

Add compatibility with LLVM 20.1 #1993

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

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

Conversation

svenstaro
Copy link
Contributor

Description

This adds LLVM 20 compatibility. The changes are thankfully quite concise. Fixes #1963.

Tests

Checklist:

  • I have read the contribution guidelines.
  • I have updated the documentation, if applicable.
  • I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
  • My code follows the prevailing code style of this project. If I haven't
    already run clang-format v17 before submitting, I definitely will look at
    the CI test that runs clang-format and fix anything that it highlights as
    being nonconforming.

Copy link

linux-foundation-easycla bot commented Jun 5, 2025

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: svenstaro / name: Sven-Hendrik Haase (c53e766)
  • ✅ login: lgritz / name: Larry Gritz (07d53bc)

Signed-off-by: Sven-Hendrik Haase <svenstaro@gmail.com>
@lgritz
Copy link
Collaborator

lgritz commented Jun 5, 2025

Is that really all it takes to get fully LLVM 20 enabled? How did you test it?

I tried your patch on my end using the llvm 20 from Homebrew, and I'm still getting errors like

/Users/lg/code/osl/osl.lg/src/liboslexec/llvm_util.cpp:3659:53: error: 'getDeclaration' is deprecated: Use getOrInsertDeclaration instead [-Werror,-Wdeprecated-declarations]
            llvm::Function* func = llvm::Intrinsic::getDeclaration(
                                                    ^~~~~~~~~~~~~~
                                                    getOrInsertDeclaration

@robertkirkman
Copy link

Thank you, this patch does work for me on Termux (a subset of Android 7.0), but I do also see the warning mentioned by lgritz.

/data/data/com.termux/files/home/.termux-build/openshadinglanguage/src/src/liboslexec/llvm_util.cpp:3659:53: warning: 'getDeclaration' is deprecated: Use getOrInsertDeclaration instead [-Wdeprecated-declarations]
 3659 |             llvm::Function* func = llvm::Intrinsic::getDeclaration(
      |                                                     ^~~~~~~~~~~~~~
      |                                                     getOrInsertDeclaration
/data/data/com.termux/files/usr/include/llvm/IR/Intrinsics.h:100:3: note: 'getDeclaration' has been explicitly marked deprecated here
  100 |   LLVM_DEPRECATED("Use getOrInsertDeclaration instead",
      |   ^
/data/data/com.termux/files/usr/include/llvm/Support/Compiler.h:234:50: note: expanded from macro 'LLVM_DEPRECATED'
  234 | #define LLVM_DEPRECATED(MSG, FIX) __attribute__((deprecated(MSG, FIX)))
      |                                                  ^

It might be just luck, or subtle variations between LLVM platforms, that applying this patch allows the build to complete for some people but not others, because, even though svenstaro might be using -DSTOP_ON_WARNING=OFF (observed here), in my case -DSTOP_ON_WARNING=OFF does not seem necessary to build successfully. It is possible that additional changes might be necessary before all LLVM platforms, including Homebrew/MacOS, are supported by this PR.

@svenstaro
Copy link
Contributor Author

Is that really all it takes to get fully LLVM 20 enabled? How did you test it?

I built and used blender against it.

I tried your patch on my end using the llvm 20 from Homebrew, and I'm still getting errors like

/Users/lg/code/osl/osl.lg/src/liboslexec/llvm_util.cpp:3659:53: error: 'getDeclaration' is deprecated: Use getOrInsertDeclaration instead [-Werror,-Wdeprecated-declarations]
            llvm::Function* func = llvm::Intrinsic::getDeclaration(
                                                    ^~~~~~~~~~~~~~
                                                    getOrInsertDeclaration

You can try to build without -Werror. I didn't fix all the deprecation warnings as I wasn't sure that effect it would have on other LLVM versions. I wanted to keep my patch minimal.

@lgritz
Copy link
Collaborator

lgritz commented Jun 7, 2025

I believe I have a fix. @svenstaro do you mind if I push an update to your fork to complete this PR?

@svenstaro
Copy link
Contributor Author

Please go right ahead.

Signed-off-by: Larry Gritz <lg@larrygritz.com>
@lgritz
Copy link
Collaborator

lgritz commented Jun 7, 2025

With the additional changes I just made, it all seems to build and work properly when tested on my Mac with llvm20 from Homebrew.

@lgritz
Copy link
Collaborator

lgritz commented Jun 9, 2025

@svenstaro Please let me know if the changes I pushed look ok to you. We don't like to push to somebody else's PR and then merge it with their name attached if we aren't sure that they fully agree with the changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LLVM 20 compatibility
3 participants