[CMake][Doc] Document BUILD_DUMMY_LIBTVM and source-build for compilation - #3493
[CMake][Doc] Document BUILD_DUMMY_LIBTVM and source-build for compilation#3493nuri-yoo wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a status message in CMakeLists.txt to inform users when BUILD_DUMMY_LIBTVM is enabled and updates the documentation with instructions for building an LLVM-enabled libtvm for model compilation. Feedback was provided regarding the implementation of BUILD_DUMMY_LIBTVM as a hardcoded variable, which prevents users from overriding it via the command line, and a correction was suggested for the documentation to ensure the USE_LLVM flag is passed correctly to CMake without premature shell substitution.
| message(STATUS "BUILD_DUMMY_LIBTVM=ON: libtvm will be a runtime-only stub. " | ||
| "Sufficient for loading prebuilt rt.dylib artifacts. " | ||
| "To build a libtvm capable of `mlc_llm compile`, re-configure " | ||
| "with -DBUILD_DUMMY_LIBTVM=OFF -DUSE_LLVM=<llvm-config path>.") |
There was a problem hiding this comment.
The status message informs users that they can re-configure with -DBUILD_DUMMY_LIBTVM=OFF. However, line 58 uses set(BUILD_DUMMY_LIBTVM ON), which creates a normal variable that shadows any cache variable set via the command line (e.g., using -D). This means that passing -DBUILD_DUMMY_LIBTVM=OFF to cmake will have no effect on the build, as the local variable will override the cache value. To make this "escape hatch" functional, line 58 should be changed to an option() or a cache variable.
| # Linux (apt) | ||
| sudo apt install llvm-dev | ||
| cmake .. -DBUILD_DUMMY_LIBTVM=OFF \ | ||
| -DUSE_LLVM="$(llvm-config --link-static)" |
There was a problem hiding this comment.
The command $(llvm-config --link-static) uses shell substitution syntax which executes the command in the shell before passing the result to cmake. Since llvm-config --link-static without other flags (like --libs) typically produces no output, the USE_LLVM variable will be set to an empty string. To correctly pass the command string to CMake (which TVM's build system expects so it can handle the flags itself), it should be provided as a literal string.
| -DUSE_LLVM="$(llvm-config --link-static)" | |
| -DUSE_LLVM="llvm-config --link-static" |
…tion Source builds default to BUILD_DUMMY_LIBTVM=ON, producing a runtime- only libtvm that is fine for loading prebuilt rt.dylib artifacts but silently breaks `mlc_llm compile` later (LLVM target codegen is not linked, so calls like `tvm.codegen.llvm.GetDefaultTargetTriple` fail with an opaque "function not registered" error). The default is discoverable nowhere — neither in CMakeLists.txt comments, gen_cmake_config.py prompts, nor docs/install/mlc_llm.rst. Two minimal additions: - CMakeLists.txt: emit a `message(STATUS ...)` right after the default `set(BUILD_DUMMY_LIBTVM ON)` so configure-time output tells the user the libtvm being built is runtime-only and how to switch (-DBUILD_DUMMY_LIBTVM=OFF -DUSE_LLVM=<llvm-config path>). - docs/install/mlc_llm.rst: add a `.. note::` under Step 2 of the source build instructions with the matching cmake invocation for macOS (Homebrew LLVM) and Linux (apt), plus a quick `nm | grep llvm` check to confirm LLVM ended up in the resulting libtvm. No behavioural change; the default stays BUILD_DUMMY_LIBTVM=ON. Anyone wanting compilation support flips the two flags as documented.
d271590 to
074b9ba
Compare
|
Both review points addressed in 074b9ba:
|
Source builds default to
BUILD_DUMMY_LIBTVM=ON, which produces a runtime-onlylibtvmthat loads prebuiltrt.dylibartifacts but silently breaksmlc_llm compilelater — LLVM target codegen is not linked, so calls liketvm.codegen.llvm.GetDefaultTargetTriplefail with an opaque "global function not registered" error after the user has finished a long build. Today the default is discoverable nowhere: not inCMakeLists.txtcomments, not incmake/gen_cmake_config.pyprompts, not indocs/install/mlc_llm.rst(which only mentions the TVM install doc indirectly).This change makes the default and its escape hatch visible at the two earliest natural points.
CMakeLists.txtemits amessage(STATUS ...)immediately after the existingset(BUILD_DUMMY_LIBTVM ON)so anyone runningcmake ..sees what the default produces and how to switch (-DBUILD_DUMMY_LIBTVM=OFF -DUSE_LLVM=<llvm-config path>); the message uses the same form as the surroundingmessage(STATUS ...)calls (lines 23, 40, 66, 124).docs/install/mlc_llm.rstgains a.. note::under "Option 2. Build from Source / Step 2" with the matching cmake invocation for macOS (Homebrew LLVM) and Linux (apt), plus annm build/tvm/libtvm.{dylib,so} | grep -ci llvmcheck to confirm LLVM ended up in the resulting libtvm.No behavioural change;
BUILD_DUMMY_LIBTVM=ONstays the default — it is the right default for the dominant runtime-only use case (prebuilt wheel users). Anyone wanting compilation support flips the two flags as documented.