Repository navigation
feature: make LambdaInstUseVisitor clobbering aware - #526
Open
johnbowen42 wants to merge 47 commits into
Open
johnbowen42 wants to merge 47 commits into
johnbowen42 wants to merge 47 commits into
Conversation
johnbowen42
added this pull request to stack #527
September 16, 2026 20:08
johnbowen42
force-pushed
the
feature/bowen/clobbering-aware-use-analysis
branch
from
September 16, 2026 20:10
37dfebc to
25cb26d
Compare
Base automatically changed from
feature/bowen/mfem-raja-analysis-changes
to
main
September 16, 2026 22:23
johnbowen42
force-pushed
the
feature/bowen/clobbering-aware-use-analysis
branch
2 times, most recently
from
September 17, 2026 18:37
e1298c3 to
ad2d3ee
Compare
johnbowen42
force-pushed
the
feature/bowen/clobbering-aware-use-analysis
branch
from
September 24, 2026 00:09
ad2d3ee to
976c996
Compare
johnbowen42
force-pushed
the
feature/bowen/clobbering-aware-use-analysis
branch
from
October 12, 2026 19:22
c328997 to
5091dc4
Compare
johnbowen42
added a commit
that referenced
this pull request
Oct 14, 2026
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
johnbowen42
force-pushed
the
feature/bowen/clobbering-aware-use-analysis
branch
2 times, most recently
from
October 15, 2026 02:45
c465b25 to
3a50d90
Compare
johnbowen42
added a commit
that referenced
this pull request
Oct 15, 2026
johnbowen42
force-pushed
the
feature/bowen/clobbering-aware-use-analysis
branch
from
October 15, 2026 17:27
3a50d90 to
34051bc
Compare
…ment patterns of visitor
…iling due to recognition of various nocapture intrinsics getting marked as clobbers
81 - lambda_call_shapes.HIP (Failed)
82 - lambda_memory_intrinsic_shapes.HIP (Failed)
83 - lambda_pointer_field_load.HIP (Failed)
84 - lambda_clobber_shapes.HIP (Failed)
85 - lambda_clobber_adversarial.HIP (Failed)
90 - lambda_inst_visitor_adversarial.HIP (Failed)
91 - lambda_inst_visitor_call_context.HIP (Failed)
92 - lambda_inst_visitor_call_clobber.HIP (Failed)
lambda_interior_slot_overwrite re-stores a two-slot functor's first slot from the authoritative source and overwrites the second slot from a donor functor before the call. The analysis anchors its reconciliation query at the narrow slot-0 store, so the slot-1 write is invisible: it does not cover the tracked byte (candidate collection skips its GEP) and does not alias the 4-byte anchor once SROA gives both stores one provable base. The analysis concludes the whole functor is authoritative from Initial and specializes; slot 1 executes with Donor's value, so both FileCheck assertions fail (a LambdaSpec line appears against CHECK-NOT, and Y prints 757 instead of 999). This FAILS until functor-extent tracking lands (gate item 8 in docs/dev/reviews/clobbering-aware-use-analysis.md): the query and candidate coverage must reason about [offset, offset + sizeof(functor)), not byte 0. Note the optnone variant of this shape declines only by accident -- spill reloads give the two slot stores unrelatable bases, so AA conservatively aliases the anchor -- which is the anchor-luck failure mode the review describes.
…ysis The wrapper operator's proteus.wrapper_call annotation gains a second payload: sizeof the registered closure, folded per-target by Sema at template instantiation, immune to IR type erasure. Both compilations parse it and stamp proteus.functor_size metadata on the wrapper operator; on the host, attachFunctorSizeToCallsiteMetadata recomputes the size from the registered functor type, so setU64FunctionMetadata's conflict fatal doubles as a host/device layout cross-check. The device manifest requires the metadata and passes the extent to analyzeLambdaUses.
Three size-based checks, each pinned by a test, none touching witness resolution: (a) the fallback MemorySSA query location is bounded to the prefix [0, tracked offset + size), so writes past the functor (envelope suffixes, enclosure trailing captures) can no longer be selected as the clobber and spuriously decline correct shapes; (b) a transfer that covers the tracked byte must cover the functor's full extent or decline, in both visitors -- a shorter copy (the classic wrong-sizeof bug, legal C++ with no casts) splices two sources into one functor and the byte-0 witness would lie for the uncovered slots; (c) memset declines on any overlap with the extent instead of ignoring interior-only hits. The precise zero-remaining-GEP location is deliberately NOT widened to the extent: interior slot stores are legitimate parallel copies in slot-wise assembly (the mfem shape), and extent-wide visibility requires per-piece reconciliation to accept them. Interior-store soundness is a frontend contract; see gate 8 in docs/dev/reviews/clobbering-aware-use-analysis.md. Verified via the pseudo-GPU harness on LLVM 19: all 16 lambda suites decision-identical to the pre-change baseline, and lambda_size_test's envelope-suffix case flips from spurious decline to specialize.
lambda_size_test pins the two extent behaviors: the enclosure shape (slot-wise assembly with a trailing capture past the functor) must specialize, and the envelope-suffix shape must specialize rather than spuriously decline when zero-index GEPs fold at -O1 and the suffix store would otherwise be selected as the clobber. The review doc records the resolved gate-8 design and the remaining frontend-diagnostic work item.
Records the open design decision underlying the interior-slot/mfem tension: specialization needs every registered slot's reaching definition to trace to one kernel argument, but the analysis resolves a single witness chain, so a provenance horizon must be chosen -- assume parallel copies behind a frontend contract (A, current), prove a common source within the assembling frame (B, loses mfem/RAJA whose pieces arrive SROA-decomposed), or merge per-piece kernel-arg resolutions (C, complete but N backward walks per callsite). Pins the decision drivers, the current position, and every artifact that must change if the position changes. Adds a Design Decisions section to the developer manual nav.
The kernel-arg merge (C) re-proves what A assumes rather than accepting anything new: mfem already specializes under the witness model, and the shapes C would newly reject -- closures dissected and reassembled from different kernel arguments through type erasure -- are written by no portability layer, forbidden by the usage contract, and visible to the AST checker, which is the cheaper and sharper enforcement point. Records the PR-level framing: the value of this work is the change in failure category (miscompile -> conservative decline/frontend error), not new acceptance.
Records the target IR shape (partial closure writes tracing to different kernel arguments past an accepting byte-0 witness), why conforming C++ cannot emit it (closure storage is writable only through whole-object special members; field-granular heterogeneity requires naming interior bytes, which requires type erasure), and the two probes: the union pun -- the only cast-free storage aliasing -- declines in both variants (MemoryPhi on the conditional path, unresolvable candidate set on the unconditional one) and is structurally whole-object either way; the aggregate-of-closures member splice correctly specializes, confirming the analysis keys on the closure, not its container. Fixes the AST diagnostic's scope as the exact erasure residue.
tests/gpu/lambda-spec-groundtruth.json records per-test specialize/decline decisions parsed from the ProteusPass debug trace; tools/lambda-spec-ci/lambda_spec_ci.py captures, checks, and renders it. The trace lines are identical for ROCm and pseudo-GPU runs, so one truth file serves both; CI can fail on any decision drift. Initial capture is from the pseudo-GPU pipeline and is marked as requiring ROCm re-capture before being treated as hardware ground truth.
One source base is not sufficient for multi-store reconciliation: a field-swapping copy from a single source (dest slot 0 <- src slot 1, dest slot 1 <- src slot 0) passed the same-base check and specialized transposed slots. Each candidate's (source offset - destination offset) must now equal the selected clobber's delta; mismatches decline. Verified with the lambda-spec ground-truth checker: zero decision drift across all 38 lambda GPU tests (pseudo-GPU pipeline, LLVM 19).
Three diagnostics implement the frontend soundness boundary decided in docs/dev/design/lambda-closure-provenance.md -- the IR analysis's witness model assumes closures are written only as whole objects, and these are exactly the type-erasure constructs that can break that assumption: - reinterpreting storage that contains a registered wrapper (directly or nested by value) through a type-erased mutable pointer; const reads stay legal. - memcpy/memmove/memset builtins whose destination is wrapper-containing storage (the wrong-sizeof partial copy needs no casts; the IR guards decline the shapes they can see, the frontend rejects them all). - unions with a wrapper-containing member. lambda_interior_slot_overwrite is retired from the GPU suite: its shape is now a compile error, covered by tests/ast/lambda_wrapper_storage_invalid.cpp together with partial-memcpy, envelope-memset, const-read-allowed, and union-pun cases. Ground truth updated (37 tests); spec-CI clean; all six existing AST tests and the mfem/factory/size-test GPU positives show no new diagnostics.
Removes code orphaned by the clobbering redesign, none of it referenced anywhere in the tree: the reaching-pointer-stores family and getCallBaseIdentityArgOperand/getRetInst in KernelArgVisitor.h (superseded by the MemorySSA oracle), and needsDefUseAnalysis, the DominatorTree member, isSelectedClobber with its commented-out call, the write-only StoreInstToOffsetBasePtrMap, and the std::variant sketch in KernelArgPtrUseVisitor.h. getReturnInstructions in PointerClobberAnalysis.h becomes inline (non-inline definition in a header; ODR hazard on a second include site). Verified: both plugins build on LLVM 19 and 22; lambda-spec ground-truth check reports zero decision drift across all 37 tests.
…offset The paired AnalysisFailed/AnalysisSuccess writes (47 sites across both visitors) collapse into a fail() helper, making the never-both-true invariant structural; several sites that set only the failure flag now clear the success flag too. The ptr-use visitor's visitor-global Offset was write-only after the range refactor -- its one consumer was the CallBase candidate's OffsetOfWriteToBasePtr, which the resolution path never reads for calls (a selected call resumes interprocedural traversal) -- so the member, its GEP accumulation, and the unused back() helper are gone. offsetValueMapFailure is renamed rangeMapFailure to match the ValueRangeMap it actually reports on. Verified: both plugins build (LLVM 19); lambda-spec ground truth reports zero decision drift across 37 tests; the AST rejection suite passes.
The decision-drift checker, ground-truth JSON, and its doc ship with the verification-tooling PR (feature/bowen/pseudo-gpu-harness) so reviewers of that small PR get the whole apparatus, and this branch's PR consumes it once landed. The branch-tip decision capture will be re-established from a ROCm run as part of final validation.
johnbowen42
force-pushed
the
feature/bowen/clobbering-aware-use-analysis
branch
from
October 15, 2026 20:11
d2b9335 to
fd0f564
Compare
Its durable content lives on: the resolved design in docs/dev/design/lambda-closure-provenance.md, the behavioral baseline in the spec-CI ground truth (tooling branch), and the fixes themselves in history. The finding-by-finding ledger was a working document for the review cycle, now stale.
…n mode The first ROCm run of the contract diagnostics rejected the analysis suite's own provenance tests: the checks were type-level overapproximations. Narrowed to what the design contract actually forbids: - memcpy/memmove: diagnose only writes that PARTIALLY overlap the wrapper's byte interval (computed from the destination expression's member chain and record layout) or whose dynamic length cannot be proven to miss it. Whole-wrapper copies and non-overlapping prefix/suffix writes are the whole-object discipline itself and stay legal. memset stays strict on any overlap. - reinterpret_cast: recording the cast is no longer the error; only a WRITE through the erased pointer (directly or via a variable initialized from it) is diagnosed, scanned post-traversal. container_of-style address math and const reads stay legal. New plugin arg -contract-warn downgrades the contract diagnostics to warnings; the Proteus test tree passes it so adversarial tests that deliberately violate the contract can keep pinning the IR analysis's conservative declines, while consumer builds via add_proteus() keep hard errors. The AST rejection test gains positive cases for every legal form. Verified: wrapper-storage AST test passes strict; all six prior AST tests pass; the full GPU lambda suite compiles with zero contract errors under -contract-warn.
johnbowen42
marked this pull request as draft
October 15, 2026 21:47
Collaborator
Author
|
marking as draft temporarily while I push up several commits |
johnbowen42
marked this pull request as ready for review
October 16, 2026 16:05
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Currently if we have LLVM IR shapes like
our analysis of the lambda callsite cannot automatically tell which store to trust as the definitive source of truth for the first slot of the lambda at the callsite. Fortunately, LLVM has O(N) (N is size of the function) built-in analysis called MemorySSA, which can answer, at the callsite
call registered_lambda(%0), which store is definitive. In the parlance of MemorySSA, we say the instructionstore int32 %2, %0"clobbers", where clobbering basically means is the source of truth for that memory value and the callsite.Fortunately, this LLVM IR shape is relatively exotic for lambda closure pointer. Nonetheless, we want Proteus to conservatively and correctly deal with simple cases of memory clobbering, explicitly reject complex LLVM IR shapes, and above all not trust the def-use graph traversal ordering as a source of ground truth for memory locations. This PR introduces a MemorySSA oracle model, that the def-use walk uses as a guide when analyzing instructions that write to lambda closure pointers. The oracle allows the analysis to ask, "which instruction clobbers the lambda closure pointer at a given lambda callsite??" Of course, an adversarial user can cook up examples where lambda closures pass through phi nodes, or a lambda is re-assembled piecemeal from disparate memory locations. These are all cases we currently conservatively reject, and update the clang frontend to do help users understand why.
Here are some of the edge cases where we fail to specialize
(1) What if the analysis finds that different slots in a lambda are stored from different pointers? -- fail to specialize
(2) What if we find that multiple user-defined function calls determine the value of different slots of the lambda closure -- fail to specialize
(3) What if we find multiple memcpys that copy different lambda slots from different source pointers? -- fail to specialize
Here are some cases where we do specialize
✅ the lambda closure pointer is clobbered at the callsite by a collection of stores, all from the same base ptr
✅ the lambda closure pointer is stored to multiple times, only one store clobbers
✅ the lambda closure pointer is clobbered at the callsite by a memory intrinsic
✅ the lambda closure pointer is clobbered at the callsite by a function call, which writes to the closure pointer using one of the above patterns
We try to handle all unsupported LLVM IR patterns from the Clang frontend which is also updated here. I also have added a flag to weaken the Clang frontend so that we can test that the LLVM IR analysis also knows about the bad shapes I describe above.