[IR Container] Phase 3 Draft#6030
Draft
mdavis36 wants to merge 6 commits intomd/segmenter-container-sharingfrom
Draft
[IR Container] Phase 3 Draft#6030mdavis36 wants to merge 6 commits intomd/segmenter-container-sharingfrom
mdavis36 wants to merge 6 commits intomd/segmenter-container-sharingfrom
Conversation
Collaborator
Author
|
!test |
When a Fusion is destroyed, removeStatementsOwnedBy frees its Exprs but previously did not clean up uses_/definition_ pointers on Vals. Shared scalars that survive via the multi-owner guard retained dangling Expr pointers in their uses_ vectors, causing heap corruption. Four fixes: - Swap Expr/Val processing order in removeStatementsOwnedBy so Exprs are cleaned up before Vals, and call removeUse()/setDefinition(nullptr) before freeing each Expr (matching removeExpr() pattern) - Add multi-owner guards to removeVal, removeStatementsCreatedAfter, and the erase_if path so shared Vals are not freed while other Fusions still reference them - Skip addUse() for shared scalars in registerExpr and clear uses_ when a Val transitions to shared (addOwningFusion), preventing cross-Fusion DAG leakage - Remove fatal NVF_ERROR in Val::uses() for shared scalars — they now correctly return an empty vector
Remove the debug guard (if false &&) and add two exclusion conditions identified during Phase 3 testing: - !isFusionInput(): Fusion input scalars (pad widths, reshape sizes) need per-Fusion argument binding and must not be shared - !uses().empty(): Orphaned scalars (extent Vals replaced by DynamicTransform concretization) have broken evaluation chains and must not be shared
Collaborator
Author
|
!test |
1 similar comment
Collaborator
Author
|
!test |
Four code paths created Fusions with fresh IrContainers (default constructor) instead of sharing the source Fusion's container. This broke the getCurFusion()-based traversal in iter_visitor.cpp which assumes all Vals are in the same shared container. Changes: - fusion_segmenter.cpp: Welford translation test copy now shares the source Fusion's IrContainer - host_ir/container.h: Add shared-container constructor to HostIrContainer (forwarding to Fusion's protected constructor) - communication_executor.cpp, host_ir/lowering.cpp, host_ir/lower.cpp: Use shared-container constructor for HostIrContainer creation
Collaborator
Author
|
!test |
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
1 participant
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.
No description provided.