[Caching] Ref 7: CPU per-task artifact cache (ORC object serialize/load) - #901
[Caching] Ref 7: CPU per-task artifact cache (ORC object serialize/load)#901hughperkins wants to merge 16 commits into
Conversation
Extend the cross-process per-task artifact tier (6a) to the CPU backend. On a cache miss the per-task module is compiled to a host relocatable object via orc::SimpleCompiler and stored (object bytes + launch metadata) under the task's IR key; on a hit the cached object is loaded straight into the ORC object layer. Each task gets its own JITDylib (CPU analog of CUDA's one-CUmodule-per-task), so per-task objects never collide on shared symbols and no relink is needed. - codegen.cpp: widen the artifact-tier + per-task build gates to arch_is_cpu (CPU only when the tier is on; else keeps the whole-kernel module). - jit_cpu.cpp: JITSessionCPU::add_module_per_task fill+load site, JITModuleCPU multi-dylib resolve, host PIC object compile helper. - cpu/kernel_launcher.cpp: take the per-task path when per_construct_artifacts is non-empty (mirror of the CUDA launcher). - llvm_program.cpp: scope the per-task dir by host triple+CPU (as CUDA scopes by sm) so an NFS-shared cache never serves an incompatible host object. - test_per_offload_cache.py: CPU cross-process reuse + disabled-tier tests.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7653c8f540
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…(codex) A decodable .qdb record whose object bytes are malformed (e.g. disk corruption) reaches object_layer_.add, which parses eagerly; the previous cantFail would abort the whole process on every launch through that cache path. Raise a catchable QD_ERROR instead (mirroring the CUDA per-task load's QD_ERROR_IF) and drop the offending entry so a later process recompiles and refills it. Adds PerTaskArtifactCache::erase for the invalidate step.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8153378b71
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
… (codex) The per-task artifact cache is no longer CUDA-only; note that it also applies to CPU (host object payload) and that it is scoped per target device.
|
@codex review |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e9ea9a02a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…s rejected (codex) On the corrupt-object error path, advance module_counter_ before creating the dylib so the lingering (empty) dylib left behind after the throw can't collide with a later per-task/whole-module name (which would fail createJITDylib). Also erase the on-disk entry on both the hit and miss paths (either may have written one), so a bad freshly-compiled object never poisons the cache.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 57d654cb76
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…te (codex) object_layer_.add only registers a materialization unit, so it catches object parse errors but not a corrupt relocation / bad reference, which fails later during linking -- at launch-time lookup, where no key is available to invalidate the record. Force materialization right after add (resolve each task's entry symbol in its self-contained dylib) so a deferred link failure is caught while art.key is still in scope, erased, and raised catchably instead of poisoning every future process.
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
…argon compile_module_to_object built the TargetMachine from the host CPU *name* with an empty feature string, which selects that CPU model's default feature set -- a superset of what the running core may actually enable -- so the emitted per-task object could use instructions the host lacks and crash with SIGILL at kernel launch (seen on some CI runners). Build the target machine from detectHost()'s JITTargetMachineBuilder instead, carrying the explicit detected host features, exactly as the whole-kernel ConcurrentIRCompiler(JTMB) path does. Also reword the init_options per-task cache note to drop undefined jargon (PTX / compute capability) that failed the doc-quality check.
# Conflicts: # docs/source/user_guide/init_options.md # quadrants/codegen/codegen.cpp # quadrants/runtime/program_impls/llvm/llvm_program.cpp # tests/python/test_per_offload_cache.py
|
CI was green prior to merge |
|
So we have:
|
|
Ci ~green |
Drop the parenthetical detailing per-backend payloads and per-target scoping from the init_options per-task cache bullet.
Shorten the per-task cache comments to only note the non-obvious bits (one dylib per task, forced materialization, host-feature detection); drop restatements of the code and cross-backend history.
| # fill it, and it is gated on `offline_cache`. Reuse is reported on `PerOffloadCacheObservations.tasks_*` (-1 when the | ||
| # tier did not run). | ||
| # task's own IR (name-free), so a later process reuses an unchanged task instead of recompiling it. CUDA and AMDGPU fill | ||
| # it with GPU code and CPU (ref 7) with a host object; it is gated on `offline_cache`. Reuse is reported on |
There was a problem hiding this comment.
dont mention ref 7. No-one knows what that is.
There was a problem hiding this comment.
Agent says:
Done in 28dfae4 -- dropped "(ref 7)" from the comment. It now reads "... and CPU with a host object; ...".
Say "offline_cache is enabled" instead of "the tier is on" so the comments are readable without knowing the internal cache vocabulary.
Clearer name for the flag that gates the per-task artifact cache.
Clearer name for the per-task cache eligibility flag.
…tion Rename widened the first line, so re-indent the aligned continuation.
The comment should read without knowing internal PR numbering.
|
running genesis unit tets. on tests0 |
|
running gen bench on bench1 |
1 participant
Summary
Extends the cross-process per-task compile-cache tier (6a, #893) to the CPU backend. Ref 7 is a fill + assemble/load adapter onto 6a's existing seam: everything 6a built (per-task IR key, on-disk
PerTaskArtifactstore, the probe / eligibility gate /.qdcper_task_artifact_keyspersistence /tasks_*observability incodegen.cpp) is backend-agnostic and reused unchanged. The only new backend payload is a host relocatable object where CUDA's is PTX. Depends on 6a (#893) and 1c (#864), both merged; 1b is not a dependency (CPU needs no relink).codegen/codegen.cpp— widen the twoarch == Arch::cudagates toarch_is_cpu. CPU takes the per-task path only when the artifact tier is on (artifact_tier); withoffline_cache=falseit keeps its existing whole-kernel module. CUDA behavior is unchanged.runtime/cpu/jit_cpu.cpp— implementJITSessionCPU::add_module_per_task(mirror of the CUDA fill site). Miss: compile the per-task module to a host object viaorc::SimpleCompiler(host PIC target machine, matchingKernelCodeGenCPU::optimize_module) and store{object bytes, tasks, used_tree_ids, struct_for_tls_sizes}under the IR key. Hit: wrap the cached bytes in aMemoryBufferand add to the object layer. OneJITDylibper task (CPU analog of CUDA's one-CUmodule-per-task), so per-task objects can never collide on a shared helper/global symbol — which is why CPU needs no 1b-style relink.JITModuleCPUnow resolves symbols across its N dylibs.runtime/cpu/kernel_launcher.cpp— take the per-task path whenper_construct_artifactsis non-empty (mirror of the CUDA launcher), else the whole-kernel module.runtime/program_impls/llvm/llvm_program.cpp— scope the per-task dir by host triple + CPU name (as CUDA scopes bysm_<cc>), so an NFS-shared cache on a heterogeneous cluster never serves a host object to an incompatible CPU.tests/python/test_per_offload_cache.py— CPU cross-process reuse test + CPU disabled-tier test.Note: BLS (
bls_buffer,mem_access_opt) is GPU-only, so thecodegen_cpu.cpplinkage question from the plan is moot on CPU —create_bls_buffernever fires there.Test plan
Built and run on the cluster (x64, LLVM 22.1.0). All green:
test_per_offload_cache(cpu): 22 passed, incl.test_per_task_artifact_cache_reuses_shared_task_cross_process_cpu(a fresh process loads the unchanged task's object from disk, recompiles only the differing task, byte-identical result) andtest_per_task_artifact_cache_disabled_without_offline_cache_cpu.offline_cache=True):offline_cache26,struct_for14,ad_basics54 (exercises eligibility-excluded adstack tasks),atomic35,reduction16,element_wise16,listgen2 — all pass.codegen.cppchange is a trivial gate widening with no CUDA code touched).Made with Cursor