Skip to content

[FEA] Remove precompiled header directory specifications - #23987

Merged
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
lamarrr:pch-lto-fix
Sep 8, 2026
Merged

[FEA] Remove precompiled header directory specifications#23987
rapids-bot[bot] merged 1 commit into
NVIDIA:mainfrom
lamarrr:pch-lto-fix

Conversation

@lamarrr

@lamarrr lamarrr commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Description

Follow-up to #22812
Removes manual PCH cache directory specification for LTO fragment compilation.
It was already removed for the pure CUDA-only path.

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@lamarrr
lamarrr requested a review from a team as a code owner September 4, 2026 18:07
@lamarrr lamarrr added feature request New feature or request non-breaking Non-breaking change labels Sep 4, 2026
@github-actions github-actions Bot added the libcudf Affects libcudf (C++/CUDA) code. label Sep 4, 2026
@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: b4e0f508-fdec-4d98-beee-0f806acbed1f

📥 Commits

Reviewing files that changed from the base of the PR and between cb88f9b and ccec3ae.

📒 Files selected for processing (3)
  • cpp/src/jit/cache.cpp
  • cpp/src/runtime/context.cpp
  • cpp/src/runtime/context.hpp
💤 Files with no reviewable changes (3)
  • cpp/src/jit/cache.cpp
  • cpp/src/runtime/context.cpp
  • cpp/src/runtime/context.hpp

Included review availability: Your plan provides up to 12 included reviews per hour; 10 remain after this review.


📝 Summary

Summary by CodeRabbit

  • Changes
    • Removed support for configuring a separate directory for just-in-time compilation precompiled headers.
    • JIT compilation no longer creates or uses a dedicated PCH cache directory.
    • Existing precompiled-header controls and verbosity options remain available and unchanged.

Walkthrough

The JIT context no longer manages a PCH directory. The fragment compiler no longer passes --pch-dir to NVRTC. Existing PCH and verbosity options remain.

Changes

JIT PCH directory removal

Layer / File(s) Summary
Remove JIT PCH context state
cpp/src/runtime/context.hpp, cpp/src/runtime/context.cpp
The context configuration and accessor no longer expose a JIT PCH directory. Initialization no longer creates or constructs the directory.
Simplify NVRTC PCH options
cpp/src/jit/cache.cpp
compile_fragment no longer retrieves a PCH directory or passes --pch-dir to NVRTC.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to ccec3

The JIT LTO compilation path no longer supplies a manual PCH directory, aligning it with the CUDA-only path. No current merge-readiness risk is identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description check ✅ Passed The description clearly explains that the PR removes manual PCH cache directory specification for LTO fragment compilation and aligns this path with the CUDA-only path.
Title check ✅ Passed The title clearly and concisely identifies the removal of precompiled header directory specifications, which is the main change.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@lamarrr lamarrr changed the title [FEA] Remove precompiled header directory references from JIT context [FEA] Remove precompiled header directory specifications Sep 4, 2026
@lamarrr

lamarrr commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot
rapids-bot Bot merged commit 34f116a into NVIDIA:main Sep 8, 2026
163 of 164 checks passed
@lamarrr
lamarrr deleted the pch-lto-fix branch September 10, 2026 10:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature request New feature or request libcudf Affects libcudf (C++/CUDA) code. non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants