[Hostjit] Move Clang/LLD hostjit into a separate library - #9583
Conversation
|
CC @gevtushenko |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Changeslibnvcc extraction and hostjit rewiring
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
c/parallel.v2/src/hostjit/codegen/bitcode.cpp (1)
125-138: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winimportant: propagate bitcode compilation failures instead of only printing to
stderrand returningfalse. The current callers ignorecompile_and_add(...)’s return value, so a libnvcc failure can silently continue without required bitcode and fail later with misleading diagnostics. As per path instructions: focus on public C API error/status handling.Source: Path instructions
c/parallel.v2/src/hostjit/CMakeLists.txt (1)
113-117: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winimportant: Exclude
libnvccand any default-built v2 consumers from MSVC Debug as well.add_subdirectory(libnvcc)runs unconditionally on Line 23, but this block only excludescccl.c.parallel.v2.hostjit_lib; a Debug build can still pulllibnvccthroughcccl.c.parallel.v2or packaging targets.c/parallel.v2/src/hostjit/libnvcc/compiler.cpp (3)
71-91: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winimportant: make LLVM initialization thread-safe. Parallel libnvcc calls can read/write
llvm_initializedconcurrently and can run LLVM target initialization twice. Usestd::once_flag/std::call_once.
1660-1666: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winimportant: do not silently skip unreadable or empty LTOIR inputs. The caller explicitly supplied these files, so an empty read should report which
--device-ltoirfailed instead of continuing with a different link input set.
1701-1704: 🗄️ Data Integrity & Integration | 🟠 Majorimportant: check the return codes from both
nvJitLinkGetLinkedCubinSizeandnvJitLinkGetLinkedCubin; on failure, stop before writingcubin_dataor passing it tonvFatbin, or this path can emit invalid cubin contents.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 428555d5-25cb-4ff8-858b-4ced8cf7bcab
📒 Files selected for processing (12)
c/parallel.v2/CMakeLists.txtc/parallel.v2/src/hostjit/CMakeLists.txtc/parallel.v2/src/hostjit/codegen/bitcode.cppc/parallel.v2/src/hostjit/config.cppc/parallel.v2/src/hostjit/include/hostjit/compiler.hppc/parallel.v2/src/hostjit/include/hostjit/config.hppc/parallel.v2/src/hostjit/include/hostjit/jit_compiler.hppc/parallel.v2/src/hostjit/jit_compiler.cppc/parallel.v2/src/hostjit/libnvcc/CMakeLists.txtc/parallel.v2/src/hostjit/libnvcc/compiler.cppc/parallel.v2/src/hostjit/libnvcc/include/libnvcc/libnvcc.hpython/cuda_cccl/CMakeLists.txt
💤 Files with no reviewable changes (1)
- c/parallel.v2/src/hostjit/include/hostjit/jit_compiler.hpp
| pch_path = get_pch_path(kind_name, config.sm_version); | ||
| if (std::filesystem::exists(pch_path)) | ||
| { | ||
| return true; | ||
| } | ||
|
|
||
| config.enable_pch = false; | ||
| config.device_pch_path.clear(); | ||
| config.host_pch_path.clear(); | ||
|
|
||
| std::vector<std::string> options; | ||
| config.appendCommandLineArguments(options); | ||
| auto option_ptrs = hostjit::detail::make_libnvcc_option_ptrs(options); | ||
|
|
||
| hostjit::detail::LibnvccProgramGuard program; | ||
| auto create_result = libnvccCreateProgram(&program.program, pch_preamble_source, "hostjit_preamble.cu"); | ||
| if (create_result != LIBNVCC_SUCCESS) | ||
| { | ||
| diagnostics += "Failed to create libnvcc PCH program: "; | ||
| diagnostics += libnvccGetErrorString(create_result); | ||
| diagnostics += "\n"; | ||
| pch_path.clear(); | ||
| return false; | ||
| } | ||
|
|
||
| auto source_path = get_pch_source_path(kind_name, config.sm_version); | ||
| auto pch_result = libnvccCreatePCH( | ||
| program.program, | ||
| kind, | ||
| source_path.c_str(), | ||
| pch_path.c_str(), | ||
| static_cast<int>(option_ptrs.size()), | ||
| option_ptrs.empty() ? nullptr : option_ptrs.data()); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
important: avoid check-then-write races for shared PCH paths. Two processes can both miss exists(pch_path), or one can observe a partially written .pch; write to a unique temp path and atomically rename or guard generation with a lock.
There was a problem hiding this comment.
I think that too should be reserved for a future patch if we care about making sure that this works properly across multiple processes
There was a problem hiding this comment.
suggestion: @aleeb-nvidia makes sense to defer this. The residual risk is limited to concurrent processes sharing hostjit_pch and observing the same *.pch path during generation. If you want, I can open a follow-up issue to track adding locking or temp-file-plus-rename around PCH creation.
🧠 Learnings used
Learnt from: sleeepyjack
Repo: NVIDIA/cccl PR: 8857
File: cudax/include/cuda/experimental/__cuco/__hyperloglog/hyperloglog_impl.cuh:426-428
Timestamp: 2026-05-06T23:47:17.597Z
Learning: In the CCCL/CUDA C++ Core Libraries codebase (NVIDIA/cccl), treat `_CCCL_API` as host-and-device callable. `_CCCL_API` expands to `_CCCL_HOST_DEVICE` via `libcudacxx/include/cuda/std/__cccl/visibility.h`, so functions marked with `_CCCL_API` may be invoked from both host and device code. When reviewing device-code contexts, do not flag `_CCCL_API`-annotated functions as if they were host-only.
Learnt from: shwina
Repo: NVIDIA/cccl PR: 9568
File: c/parallel/src/segmented_reduce.cu:599-643
Timestamp: 2026-06-23T20:49:51.585Z
Learning: When reviewing uses of NVIDIA/cccl AOT serialization, don’t flag a potential mismatch when `cccl::aot::buffer_writer::write_blob` is paired with `cccl::aot::buffer_reader::read_into(void* dest, size_t expected_size)`. The `read_into` implementation reads a length-prefixed blob: it `read_pod`s the stored length, checks it equals `expected_size`, and only then copies that many bytes into `dest`. This behavior is the intended counterpart for `write_blob`, so these pairs should be treated as compatible rather than mismatched.
| extern "C" libnvccResult libnvccCreateProgram(libnvccProgram* prog, const char* src, const char* name) | ||
| { | ||
| if (!prog || !src) | ||
| { | ||
| return LIBNVCC_ERROR_INVALID_INPUT; | ||
| } | ||
| *prog = nullptr; | ||
|
|
||
| auto* program = new libnvccProgram_st; | ||
| program->source = src; | ||
| program->name = (name && name[0]) ? name : "input.cu"; | ||
| *prog = program; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift
critical: do not let C++ exceptions cross the C ABI. new, std::string assignment, option parsing, and compilation can throw before a libnvccResult is returned, despite the API defining OOM/internal error statuses. Wrap exported entry points in noexcept exception boundaries that map std::bad_alloc and other exceptions to result codes. As per path instructions, c/**/* reviews should focus on C/C++ boundary behavior and error/status handling.
Also applies to: 2267-2390
Source: Path instructions
There was a problem hiding this comment.
To my knowledge catching bad_alloc almost never makes sense since the OS will OOM-kill the process before a memory allocation has a chance to fail (unless you try to allocate like 1TB in a single allocation or sth like that).
Is exception handling even enabled in the contexts where this library will be used? At least LLVM is generally build w/ -fno-exceptions -fno-rtti.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Locate relevant CMake files =="
fd -i 'CMakeLists.txt' c/parallel.v2/src/hostjit c/parallel.v2 | sed 's#^\./##' | sort
echo
echo "== Inspect libnvcc target CMake =="
sed -n '1,260p' c/parallel.v2/src/hostjit/libnvcc/CMakeLists.txt
echo
echo "== Inspect parent hostjit CMake =="
sed -n '1,260p' c/parallel.v2/src/hostjit/CMakeLists.txt
echo
echo "== Search for exception-related CMake settings in c/parallel.v2 =="
rg -n -C2 --glob 'c/parallel.v2/**/CMakeLists.txt' --glob 'c/parallel.v2/**/*.cmake' \
'fno-exceptions|/EH|exceptions|LLVM_ENABLE_EH|LLVM_ENABLE_RTTI|LLVM_REQUIRES_EH|CXX_EXCEPTIONS|target_compile_options|target_compile_definitions|DISABLE_RTTI|LLVM_NO_RTTI' .
echo
echo "== Search for noexcept or exception mentions in libnvcc public header/source =="
rg -n -C2 'noexcept|throw|bad_alloc|exception' \
c/parallel.v2/src/hostjit/libnvcc/include/libnvcc/libnvcc.h \
c/parallel.v2/src/hostjit/libnvcc/compiler.cppLength of output: 18839
important: @aleeb-nvidia I checked the build config. c/parallel.v2/src/hostjit/libnvcc/CMakeLists.txt only forces -fno-rtti for libnvcc; it does not disable exceptions for that target, and I didn’t find any LLVM_ENABLE_EH=OFF-style setting in this subtree either.
So LLVM itself may well be built mostly exception-free, but that doesn’t make these C wrappers safe: this TU still has its own throwing paths (new in libnvccCreateProgram, std::string/std::vector growth, filesystem/string allocations, etc.) before a libnvccResult is returned.
I do agree my original wording was too specific on bad_alloc. The real issue is the C ABI boundary, not that one exception type in particular. I’d narrow the ask to:
- either catch unexpected C++ exceptions in the exported
extern "C"entry points and map them toLIBNVCC_ERROR_OUT_OF_MEMORY/LIBNVCC_ERROR_INTERNAL_ERROR, - or explicitly define/document that these APIs may terminate on internal exceptions instead of reliably returning a status code.
So I don’t think the concern is invalid, but I would restate it more narrowly.
🧠 Learnings used
Learnt from: sleeepyjack
Repo: NVIDIA/cccl PR: 8857
File: cudax/include/cuda/experimental/__cuco/__hyperloglog/hyperloglog_impl.cuh:426-428
Timestamp: 2026-05-06T23:47:17.597Z
Learning: In the CCCL/CUDA C++ Core Libraries codebase (NVIDIA/cccl), treat `_CCCL_API` as host-and-device callable. `_CCCL_API` expands to `_CCCL_HOST_DEVICE` via `libcudacxx/include/cuda/std/__cccl/visibility.h`, so functions marked with `_CCCL_API` may be invoked from both host and device code. When reviewing device-code contexts, do not flag `_CCCL_API`-annotated functions as if they were host-only.
Learnt from: shwina
Repo: NVIDIA/cccl PR: 9568
File: c/parallel/src/segmented_reduce.cu:599-643
Timestamp: 2026-06-23T20:49:51.585Z
Learning: When reviewing uses of NVIDIA/cccl AOT serialization, don’t flag a potential mismatch when `cccl::aot::buffer_writer::write_blob` is paired with `cccl::aot::buffer_reader::read_into(void* dest, size_t expected_size)`. The `read_into` implementation reads a length-prefixed blob: it `read_pod`s the stored length, checks it equals `expected_size`, and only then copies that many bytes into `dest`. This behavior is the intended counterpart for `write_blob`, so these pairs should be treated as compatible rather than mismatched.
| @@ -0,0 +1,203 @@ | |||
| #pragma once | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
important: add the required CCCL license header before #pragma once. As per coding guidelines, **/*.{h,hpp,cuh}: “All headers must have the correct license.”
Source: Coding guidelines
There was a problem hiding this comment.
Hmm, quite a few of the headers in the hostjit library don't have licence headers; is that intentional?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Header files under c/parallel.v2/src/hostjit =="
fd -e h -e hh -e hpp -e cuh . c/parallel.v2/src/hostjit | sort
echo
echo "== First 8 lines of each hostjit header =="
while IFS= read -r f; do
echo "-- $f --"
sed -n '1,8p' "$f"
echo
done < <(fd -e h -e hh -e hpp -e cuh . c/parallel.v2/src/hostjit | sort)
echo "== First 8 lines of public C API / nearby headers under c/parallel.v2 =="
for f in \
c/parallel.v2/src/hostjit/libnvcc/include/libnvcc/libnvcc.h \
c/parallel.v2/src/hostjit/include/hostjit/compiler.hpp \
c/parallel.v2/src/hostjit/include/hostjit/config.hpp \
c/parallel.v2/src/hostjit/include/hostjit/jit_compiler.hpp
do
if [[ -f "$f" ]]; then
echo "-- $f --"
sed -n '1,8p' "$f"
echo
fi
done
echo "== Sample of license headers elsewhere in c/parallel.v2 =="
fd -e h -e hh -e hpp -e cuh . c/parallel.v2 | sort | while IFS= read -r f; do
if rg -n -m1 'SPDX-FileCopyrightText|Apache-2.0 WITH LLVM-exception' "$f" >/dev/null; then
echo "-- $f --"
sed -n '1,6p' "$f"
echo
break
fi
doneLength of output: 3012
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== First 8 lines of representative hostjit headers =="
for f in \
c/parallel.v2/src/hostjit/include/hostjit/compiler.hpp \
c/parallel.v2/src/hostjit/include/hostjit/config.hpp \
c/parallel.v2/src/hostjit/include/hostjit/jit_compiler.hpp \
c/parallel.v2/src/hostjit/include/hostjit/codegen/bitcode.hpp \
c/parallel.v2/src/hostjit/libnvcc/include/libnvcc/libnvcc.h
do
echo "-- $f --"
sed -n '1,8p' "$f"
echo
done
echo "== Count of hostjit headers with SPDX/license markers in first 8 lines =="
python - <<'PY'
from pathlib import Path
files = sorted(Path("c/parallel.v2/src/hostjit").rglob("*"))
headers = [p for p in files if p.suffix in {".h", ".hh", ".hpp", ".cuh"}]
count = 0
for p in headers:
head = "".join(p.read_text(errors="ignore").splitlines(True)[:8])
if "SPDX-FileCopyrightText" in head or "Apache-2.0 WITH LLVM-exception" in head:
count += 1
print(f"{count}/{len(headers)}")
PY
echo
echo "== Sample standard license header elsewhere in c/parallel.v2 =="
python - <<'PY'
from pathlib import Path
for p in sorted(Path("c/parallel.v2").rglob("*")):
if p.suffix not in {".h", ".hh", ".hpp", ".cuh"}:
continue
head_lines = p.read_text(errors="ignore").splitlines()[:8]
head = "\n".join(head_lines)
if "SPDX-FileCopyrightText" in head or "Apache-2.0 WITH LLVM-exception" in head:
print(f"-- {p} --")
print("\n".join(head_lines))
break
PYLength of output: 3366
important: @aleeb-nvidia it doesn’t look intentional.
I checked the hostjit subtree: it’s mixed. c/parallel.v2/src/hostjit/include/hostjit/codegen/bitcode.hpp already has the standard CCCL SPDX/license block, but c/parallel.v2/src/hostjit/include/hostjit/compiler.hpp, c/parallel.v2/src/hostjit/include/hostjit/config.hpp, c/parallel.v2/src/hostjit/include/hostjit/jit_compiler.hpp, and this new c/parallel.v2/src/hostjit/libnvcc/include/libnvcc/libnvcc.h do not. The quick count was 10/21 hostjit headers with license markers.
So I wouldn’t treat missing headers here as an intentional local convention. For this PR, I’d still expect the newly added public header to get the standard CCCL license block. If the rest of hostjit is pre-existing debt, that broader cleanup can be separate.
🧠 Learnings used
Learnt from: sleeepyjack
Repo: NVIDIA/cccl PR: 8857
File: cudax/include/cuda/experimental/__cuco/__hyperloglog/default_policy.cuh:92-95
Timestamp: 2026-05-06T23:47:10.296Z
Learning: In this CCCL (CUDA C++ Core Libraries) codebase, do NOT treat `_CCCL_API` as host-only. `_CCCL_API` is defined in `libcudacxx/include/cuda/std/__cccl/visibility.h` and always expands to include `_CCCL_HOST_DEVICE` (possibly alongside `_CCCL_TILE`, `_CCCL_VISIBILITY_HIDDEN`, and `_CCCL_EXCLUDE_FROM_EXPLICIT_INSTANTIATION`). So any function annotated with `_CCCL_API` is already valid for both host and device code; reviewers should not flag it as host-only.
Learnt from: sleeepyjack
Repo: NVIDIA/cccl PR: 8857
File: cudax/include/cuda/experimental/__cuco/__hyperloglog/hyperloglog_impl.cuh:426-428
Timestamp: 2026-05-06T23:47:17.597Z
Learning: In the CCCL/CUDA C++ Core Libraries codebase (NVIDIA/cccl), treat `_CCCL_API` as host-and-device callable. `_CCCL_API` expands to `_CCCL_HOST_DEVICE` via `libcudacxx/include/cuda/std/__cccl/visibility.h`, so functions marked with `_CCCL_API` may be invoked from both host and device code. When reviewing device-code contexts, do not flag `_CCCL_API`-annotated functions as if they were host-only.
Learnt from: NaderAlAwar
Repo: NVIDIA/cccl PR: 9171
File: libcudacxx/include/cuda/std/__cccl/diagnostic.h:126-126
Timestamp: 2026-06-03T13:24:36.356Z
Learning: When reviewing code that uses NVIDIA NVCC/NVRTC diagnostic suppression pragmas, treat `nv_diag_suppress` as supporting comma-separated warning numbers in a single directive. For example, `_Pragma("nv_diag_suppress 1444,20199")` (or an equivalent `#pragma nv_diag_suppress 1444,20199`) is valid; do not flag comma-separated warning IDs inside `nv_diag_suppress` pragmas as unsupported.
Learnt from: shwina
Repo: NVIDIA/cccl PR: 9568
File: c/parallel/src/segmented_reduce.cu:599-643
Timestamp: 2026-06-23T20:49:51.585Z
Learning: When reviewing uses of NVIDIA/cccl AOT serialization, don’t flag a potential mismatch when `cccl::aot::buffer_writer::write_blob` is paired with `cccl::aot::buffer_reader::read_into(void* dest, size_t expected_size)`. The `read_into` implementation reads a length-prefixed blob: it `read_pod`s the stored length, checks it equals `expected_size`, and only then copies that many bytes into `dest`. This behavior is the intended counterpart for `write_blob`, so these pairs should be treated as compatible rather than mismatched.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
c/parallel.v2/src/hostjit/libnvcc/compiler.cpp (1)
2283-2285: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winimportant: preserve invalid-input/option status codes at the C API boundary.
compileToDeviceBitcodeandcreatePCHcan fail fromvalidateOptions, andcreatePCHcan fail from an invalidlibnvccPCHKind, but the wrappers collapse those intoLIBNVCC_ERROR_COMPILATIONorLIBNVCC_ERROR_PCH_CREATE. Propagate a structured failure reason or validate in the wrapper before dispatch so C callers can distinguish bad inputs from compiler failures. As per path instructions,c/**/*reviews should focus on C API/ABI stability and error/status handling.Also applies to: 2382-2386
Source: Path instructions
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b190eb7d-6e0a-4971-ba29-5e6af509efcf
📒 Files selected for processing (2)
c/parallel.v2/src/hostjit/libnvcc/compiler.cppc/parallel.v2/src/hostjit/libnvcc/include/libnvcc/libnvcc.h
🚧 Files skipped from review as they are similar to previous changes (1)
- c/parallel.v2/src/hostjit/libnvcc/include/libnvcc/libnvcc.h
|
/ok to test 77b3dcc |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: cfa37b73-024b-4118-ae41-d01cc2162889
📒 Files selected for processing (11)
c/parallel.v2/CMakeLists.txtc/parallel.v2/src/hostjit/CMakeLists.txtc/parallel.v2/src/hostjit/codegen/bitcode.cppc/parallel.v2/src/hostjit/config.cppc/parallel.v2/src/hostjit/include/hostjit/compiler.hppc/parallel.v2/src/hostjit/include/hostjit/config.hppc/parallel.v2/src/hostjit/include/hostjit/jit_compiler.hppc/parallel.v2/src/hostjit/jit_compiler.cppc/parallel.v2/src/hostjit/libnvcc/CMakeLists.txtc/parallel.v2/src/hostjit/libnvcc/compiler.cppc/parallel.v2/src/hostjit/libnvcc/include/libnvcc/libnvcc.h
💤 Files with no reviewable changes (1)
- c/parallel.v2/src/hostjit/include/hostjit/jit_compiler.hpp
✅ Files skipped from review due to trivial changes (1)
- c/parallel.v2/CMakeLists.txt
🚧 Files skipped from review as they are similar to previous changes (8)
- c/parallel.v2/src/hostjit/codegen/bitcode.cpp
- c/parallel.v2/src/hostjit/config.cpp
- c/parallel.v2/src/hostjit/libnvcc/include/libnvcc/libnvcc.h
- c/parallel.v2/src/hostjit/include/hostjit/compiler.hpp
- c/parallel.v2/src/hostjit/jit_compiler.cpp
- c/parallel.v2/src/hostjit/include/hostjit/config.hpp
- c/parallel.v2/src/hostjit/libnvcc/CMakeLists.txt
- c/parallel.v2/src/hostjit/libnvcc/compiler.cpp
| set(LIBNVCC_HEADER_INSTALL_DESTINATION "cuda/cccl/headers/libnvcc") | ||
| set(LIBNVCC_CLANG_HEADER_INSTALL_DESTINATION "cuda/cccl/headers/clang") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
important: Preserve a standard install location for the new public C API header. This override replaces libnvcc’s default include/libnvcc destination with the Python header layout, so non-Python C consumers may not get <libnvcc/libnvcc.h> on the normal include path. Install to both locations or scope this override to Python packaging. As per path instructions, c/**/*: Focus on C API/ABI stability.
Source: Path instructions
…dows stack overflow) Re-target of the stack-overflow fix onto the libnvcc refactor (NVIDIA#9583): the four clang ExecuteAction calls moved from hostjit/compiler.cpp into libnvcc/compiler.cpp. Embedding clang bypasses the driver's runWithSufficientStackSpace guard; on Windows the 1 MB default stack overflows on deep template instantiation. Run each ExecuteAction on an 8 MB worker thread (= clang's DesiredStackSize).
…dows stack overflow) Re-target of the stack-overflow fix onto the libnvcc refactor (NVIDIA#9583): the four clang ExecuteAction calls moved from hostjit/compiler.cpp into libnvcc/compiler.cpp. Embedding clang bypasses the driver's runWithSufficientStackSpace guard; on Windows the 1 MB default stack overflows on deep template instantiation. Run each ExecuteAction on an 8 MB worker thread (= clang's DesiredStackSize).
…dows stack overflow) Re-target of the stack-overflow fix onto the libnvcc refactor (NVIDIA#9583): the four clang ExecuteAction calls moved from hostjit/compiler.cpp into libnvcc/compiler.cpp. Embedding clang bypasses the driver's runWithSufficientStackSpace guard; on Windows the 1 MB default stack overflows on deep template instantiation. Run each ExecuteAction on an 8 MB worker thread (= clang's DesiredStackSize).
…dows stack overflow) Re-target of the stack-overflow fix onto the libnvcc refactor (NVIDIA#9583): the four clang ExecuteAction calls moved from hostjit/compiler.cpp into libnvcc/compiler.cpp. Embedding clang bypasses the driver's runWithSufficientStackSpace guard; on Windows the 1 MB default stack overflows on deep template instantiation. Run each ExecuteAction on an 8 MB worker thread (= clang's DesiredStackSize).
…dows stack overflow) Re-target of the stack-overflow fix onto the libnvcc refactor (NVIDIA#9583): the four clang ExecuteAction calls moved from hostjit/compiler.cpp into libnvcc/compiler.cpp. Embedding clang bypasses the driver's runWithSufficientStackSpace guard; on Windows the 1 MB default stack overflows on deep template instantiation. Run each ExecuteAction on an 8 MB worker thread (= clang's DesiredStackSize).
…dows stack overflow) Re-target of the stack-overflow fix onto the libnvcc refactor (NVIDIA#9583): the four clang ExecuteAction calls moved from hostjit/compiler.cpp into libnvcc/compiler.cpp. Embedding clang bypasses the driver's runWithSufficientStackSpace guard; on Windows the 1 MB default stack overflows on deep template instantiation. Run each ExecuteAction on an 8 MB worker thread (= clang's DesiredStackSize).
…dows stack overflow) Re-target of the stack-overflow fix onto the libnvcc refactor (NVIDIA#9583): the four clang ExecuteAction calls moved from hostjit/compiler.cpp into libnvcc/compiler.cpp. Embedding clang bypasses the driver's runWithSufficientStackSpace guard; on Windows the 1 MB default stack overflows on deep template instantiation. Run each ExecuteAction on an 8 MB worker thread (= clang's DesiredStackSize).
…dows stack overflow) Re-target of the stack-overflow fix onto the libnvcc refactor (NVIDIA#9583): the four clang ExecuteAction calls moved from hostjit/compiler.cpp into libnvcc/compiler.cpp. Embedding clang bypasses the driver's runWithSufficientStackSpace guard; on Windows the 1 MB default stack overflows on deep template instantiation. Run each ExecuteAction on an 8 MB worker thread (= clang's DesiredStackSize).
…dows stack overflow) Re-target of the stack-overflow fix onto the libnvcc refactor (NVIDIA#9583): the four clang ExecuteAction calls moved from hostjit/compiler.cpp into libnvcc/compiler.cpp. Embedding clang bypasses the driver's runWithSufficientStackSpace guard; on Windows the 1 MB default stack overflows on deep template instantiation. Run each ExecuteAction on an 8 MB worker thread (= clang's DesiredStackSize).
Re-apply the device-API list from NVIDIA#9663 (which landed in main's hostjit/compiler.cpp) into jit_compiler.cpp, where the NVIDIA#9583 refactor moved the PCH preamble. Keeps the PCH cache covering all tested device APIs.
Re-apply the device-API list from NVIDIA#9663 (which landed in main's hostjit/compiler.cpp) into jit_compiler.cpp, where the NVIDIA#9583 refactor moved the PCH preamble. Keeps the PCH cache covering all tested device APIs.
Re-apply the device-API list from NVIDIA#9663 (which landed in main's hostjit/compiler.cpp) into jit_compiler.cpp, where the NVIDIA#9583 refactor moved the PCH preamble. Keeps the PCH cache covering all tested device APIs.
|
/ok to test romanso@f9ff544 |
@shwina, there was an error processing your request: See the following link for more information: https://docs.gha-runners.nvidia.com/cpr/e/2/ |
|
/ok to test 12a56b3 |
This comment has been minimized.
This comment has been minimized.
|
/ok to test 903fb73 |
This comment has been minimized.
This comment has been minimized.
|
/ok to test 1226d6e |
This comment has been minimized.
This comment has been minimized.
|
/ok to test 9712095 |
This comment has been minimized.
This comment has been minimized.
|
/ok to test 3dd411a |
🥳 CI Workflow Results🟩 Finished in 1h 33m: Pass: 100%/78 | Total: 1d 04h | Max: 59m 15s | Hits: 99%/5295See results here. |
| # Find CUDA toolkit (may already be found by parent) | ||
| if (NOT CUDAToolkit_FOUND) | ||
| find_package(CUDAToolkit) | ||
| endif() |
There was a problem hiding this comment.
No need to guard here, find_package() will exit early if it is already found
| find_file( | ||
| LIBNVCC_CPM_CMAKE_PATH | ||
| NAMES CPM.cmake | ||
| PATHS ${CMAKE_MODULE_PATH} | ||
| NO_DEFAULT_PATH | ||
| ) |
There was a problem hiding this comment.
include(CPM) (note no .cmake) will do exactly the same thing, searching CMAKE_MODULE_PATH as well as producing an error for you if it is not found.
| set(LLVM_ENABLE_PROJECTS "clang;lld" CACHE STRING "" FORCE) | ||
| set(LLVM_TARGETS_TO_BUILD "X86;NVPTX" CACHE STRING "" FORCE) |
There was a problem hiding this comment.
If the point here is to just pass these as OPTIONS in the CPMAddPackage() call, you can just do
CPMAddPackage(
OPTIONS
"LLVM_ENABLE_PROJECTS \"clang;lld\""
)to do lists. Alternatively using raw comments
CPMAddPackage(
OPTIONS
[[LLVM_ENABLE_PROJECTS "clang;lld"]]
)| message(FATAL_ERROR "libnvcc requires a CUDA toolkit") | ||
| endif() | ||
|
|
||
| target_include_directories(libnvcc PRIVATE ${CUDAToolkit_INCLUDE_DIRS}) |
There was a problem hiding this comment.
You should just target_link_libraries() here. Or is the goal to only get compile-time but no link-time dependency?
| ${llvm_project_SOURCE_DIR}/llvm/include | ||
| ${llvm_project_BINARY_DIR}/include | ||
| ${llvm_project_SOURCE_DIR}/clang/include | ||
| ${llvm_project_BINARY_DIR}/tools/clang/include | ||
| ${llvm_project_SOURCE_DIR}/lld/include | ||
| ${llvm_project_BINARY_DIR}/tools/lld/include |
There was a problem hiding this comment.
These includes should already be coming from the target_link_libraries() call you do down below, no need to add them separately.
| lldCommon | ||
| ) | ||
|
|
||
| if (CUDAToolkit_FOUND) |
There was a problem hiding this comment.
CUDAToolkit is most definitely found at this point due to the previous assertion.
| endif() | ||
|
|
||
| if (NOT LIBNVCC_HEADER_INSTALL_DESTINATION) | ||
| set(LIBNVCC_HEADER_INSTALL_DESTINATION "include/libnvcc") |
There was a problem hiding this comment.
Should use the GNUInstallDirs-provided values instead of hardcoding include/ here.
| # We DON'T install device_functions, math, or libdevice_declares — our local | ||
| # copies in cuda_minimal/ replace them. | ||
| set( | ||
| _clang_cuda_headers_needed |
There was a problem hiding this comment.
Nit: why the extra variable? You can just list these inline in the install() command, looks neater as well.
| ) | ||
| endif() | ||
|
|
||
| if (UNIX AND NOT APPLE) |
There was a problem hiding this comment.
Note the equivalent for $ORIGIN on macos is @loader_path
* [Hostjit] Move Clang/LLD hostjit into a separate library * clang-format * Address some review comments * format CMake * fix merge error * fix unused variable warning when assertions are disabled * attempt to fix DLL not found issue on Windows * run cmake formatter * attempt to fix DLL path on windows * remove invalid set_target_properties call
5 participants
Description
This moves the parts of the hostjit library that depend on Clang/LLVM/LLD/nvfatbin/nvjitlink into a separate shared library (currently called
libnvcc, though iirc we haven't decided on a name for this yet) which provides an interface similar to that of NVRTC.Any CCCL-specific parts (such as include paths as well as the PCH-caching mechanism) remain in the hostjit library and are not part of libnvcc.
This change also includes some basic refactoring in a few places in what is now libnvcc (mainly to use non-throwing filesystem APIs), but there's more refactoring to be done later that I will come back to in the future, but this change is already big enough as-is.
This is essentially a NFC: it doesn't really add or remove functionality, instead it only moves it around.
Checklist