Skip to content

[opt-viewer] Allow users to set LLVM_HAVE_OPT_VIEWER_MODULES manually… #151845

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 1 commit into
base: main
Choose a base branch
from

Conversation

kraj
Copy link
Contributor

@kraj kraj commented Aug 3, 2025

… from the CMake

Check right before setting LLVM_HAVE_OPT_VIEWER_MODULES so that if the user specifies the variable on the commandline it takes precedence. If not, fall back to the old logic

@llvmbot llvmbot added the cmake Build system in general and CMake in particular label Aug 3, 2025
kraj added a commit to kraj/llvm-project that referenced this pull request Aug 3, 2025
… from the CMake

Check right before setting LLVM_HAVE_OPT_VIEWER_MODULES
so that if the user specifies the variable on the commandline
it takes precedence. If not, fall back to the old logic

Upstream-Status: Submitted [llvm#151845]
Signed-off-by: Khem Raj <raj.khem@gmail.com>
@rossburton
Copy link

Note that the opt-viewer script is always installed anyway, even if the dependencies are not present. I'd say the same variable should be used to seed whether opt-viewer is installed or not.

kraj added a commit to kraj/llvm-project that referenced this pull request Aug 5, 2025
…he CMake

Check right before setting LLVM_HAVE_OPT_VIEWER_MODULES
so that if the user specifies the variable on the commandline
it takes precedence. If not, fall back to the old logic

Upstream-Status: Submitted [llvm#151845]

Signed-off-by: Khem Raj <raj.khem@gmail.com>
…he CMake

Check right before setting LLVM_HAVE_OPT_VIEWER_MODULES
so that if the user specifies the variable on the commandline
it takes precedence. If not, fall back to the old logic

Do not install opt-viewer of the modules are not found. Since the python
scripts have dependencies on these modules, it makes not much point to
have them installed.
@kraj kraj force-pushed the kraj/opt-viewer branch from 6856481 to 431d4ea Compare August 5, 2025 16:05
@rossburton
Copy link

I know that LLVM currently has all the detection logic embedded in AddLLVM.cmake, is that by design or just how it's happened? It feels like it could be moved into llvm/tools/opt-viewer/CMakeLists.txt which would then also automatically not run if LLVM_TOOL_OPT_VIEWER_BUILD was OFF.

kraj added a commit to kraj/llvm-project that referenced this pull request Aug 5, 2025
… from the CMake

Check right before setting LLVM_HAVE_OPT_VIEWER_MODULES
so that if the user specifies the variable on the commandline
it takes precedence. If not, fall back to the old logic

Upstream-Status: Submitted [llvm#151845]
Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj added a commit to kraj/llvm-project that referenced this pull request Aug 5, 2025
…he CMake

Check right before setting LLVM_HAVE_OPT_VIEWER_MODULES
so that if the user specifies the variable on the commandline
it takes precedence. If not, fall back to the old logic

Upstream-Status: Submitted [llvm#151845]

Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj added a commit to kraj/llvm-project that referenced this pull request Aug 6, 2025
…he CMake

Check right before setting LLVM_HAVE_OPT_VIEWER_MODULES
so that if the user specifies the variable on the commandline
it takes precedence. If not, fall back to the old logic

Upstream-Status: Submitted [llvm#151845]

Signed-off-by: Khem Raj <raj.khem@gmail.com>
kraj added a commit to kraj/llvm-project that referenced this pull request Aug 6, 2025
…he CMake

Check right before setting LLVM_HAVE_OPT_VIEWER_MODULES
so that if the user specifies the variable on the commandline
it takes precedence. If not, fall back to the old logic

Upstream-Status: Submitted [llvm#151845]

Signed-off-by: Khem Raj <raj.khem@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmake Build system in general and CMake in particular
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants