From 0a59ce93ef943ad643819f4d70cdca69835bc3a1 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Thu, 12 Feb 2026 15:29:59 -0800 Subject: [PATCH 01/10] Copy/move/swap semantics for shared containers Copy constructor now shares the source's container pointer instead of creating a new one. Fusion::copy clones directly from per-Fusion filtered vals rather than delegating to IrContainer::copy. Swap changed from content-based (IrContainer::swap) to pointer-based with per-Fusion ownership tracking for both same-container and different-container cases. --- csrc/fusion.cpp | 123 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 85 insertions(+), 38 deletions(-) diff --git a/csrc/fusion.cpp b/csrc/fusion.cpp index fb7a6a72b14..3bf05aa99f7 100644 --- a/csrc/fusion.cpp +++ b/csrc/fusion.cpp @@ -108,48 +108,89 @@ bool Fusion::sameDefinition(const Fusion& other) const { void Fusion::swap(Fusion& a, Fusion& b) noexcept { FUSER_PERF_SCOPE("Fusion swap"); - // We need to be careful to call IrContainer swap not unique_ptr swap, which - // will only swap the ptrs NOT the contents. - IrContainer::swap(*(a.ir_container()), *(b.ir_container())); + if (&a == &b) { + return; + } - // After swapping container contents, update Statement::ir_container_ - // pointers so each Statement points to the Fusion whose container now - // holds it. Also fix per-Fusion tracking keys since a's container had - // b's entries and vice versa. - a.ir_container()->transferStatementOwnership(&b, &a); - b.ir_container()->transferStatementOwnership(&a, &b); + // Collect statements owned by each Fusion BEFORE swap so we can update + // Statement::ir_container_ pointers afterward. + std::vector a_owned_vals, b_owned_vals; + std::vector a_owned_exprs, b_owned_exprs; if (a.ir_container_) { - for (auto val : a.vals()) { - val->ir_container_ = &a; - } - for (auto expr : a.deterministic_exprs()) { - expr->ir_container_ = &a; - } + const auto& av = a.ir_container_->valsOwnedBy(&a); + const auto& ae = a.ir_container_->exprsOwnedBy(&a); + a_owned_vals.assign(av.begin(), av.end()); + a_owned_exprs.assign(ae.begin(), ae.end()); } if (b.ir_container_) { - for (auto val : b.vals()) { - val->ir_container_ = &b; - } - for (auto expr : b.deterministic_exprs()) { - expr->ir_container_ = &b; - } + const auto& bv = b.ir_container_->valsOwnedBy(&b); + const auto& be = b.ir_container_->exprsOwnedBy(&b); + b_owned_vals.assign(bv.begin(), bv.end()); + b_owned_exprs.assign(be.begin(), be.end()); + } + + // Transfer Fusion registrations between containers before pointer swap. + // After swap, a will own b's container and b will own a's container. + if (a.ir_container_ && b.ir_container_ && + a.ir_container_.get() != b.ir_container_.get()) { + a.ir_container_->transferFusion(&a, &b); + b.ir_container_->transferFusion(&b, &a); } + // Swap container pointers + std::swap(a.ir_container_, b.ir_container_); + + // Swap all Fusion-level members std::swap(a.inputs_, b.inputs_); std::swap(a.outputs_, b.outputs_); - std::swap(a.io_alias_, b.io_alias_); - - // Swap per-Fusion special values (Phase 2) + std::swap(a.all_tv_uses_valid_, b.all_tv_uses_valid_); + std::swap(a.is_during_update_uses_, b.is_during_update_uses_); + std::swap(a.managed_data_, b.managed_data_); + std::swap(a.managed_named_data_, b.managed_named_data_); + std::swap(a.expected_dynamic_smem_bytes_, b.expected_dynamic_smem_bytes_); + std::swap(a.all_tvs_ptr_, b.all_tvs_ptr_); std::swap(a.zero_val_, b.zero_val_); std::swap(a.one_val_, b.one_val_); std::swap(a.true_val_, b.true_val_); std::swap(a.false_val_, b.false_val_); std::swap(a.magic_zero_val_, b.magic_zero_val_); - std::swap(a.axioms_, b.axioms_); std::swap(a.metadata_, b.metadata_); + + // Update Statement::ir_container_ pointers: a's old statements now belong + // to b, and b's old statements now belong to a + for (auto* val : a_owned_vals) { + val->ir_container_ = &b; + } + for (auto* expr : a_owned_exprs) { + expr->ir_container_ = &b; + } + for (auto* val : b_owned_vals) { + val->ir_container_ = &a; + } + for (auto* expr : b_owned_exprs) { + expr->ir_container_ = &a; + } + + // Update per-Fusion tracking keys in containers + if (a.ir_container_ && b.ir_container_) { + if (a.ir_container_.get() == b.ir_container_.get()) { + // Same container: directly swap per-Fusion tracking entries + auto* c = a.ir_container_.get(); + std::swap(c->per_fusion_vals_[&a], c->per_fusion_vals_[&b]); + std::swap(c->per_fusion_exprs_[&a], c->per_fusion_exprs_[&b]); + } else { + // Different containers: rename tracking keys to match new owners + a.ir_container_->transferStatementOwnership(&b, &a); + b.ir_container_->transferStatementOwnership(&a, &b); + } + } else if (a.ir_container_) { + a.ir_container_->transferStatementOwnership(&b, &a); + } else if (b.ir_container_) { + b.ir_container_->transferStatementOwnership(&a, &b); + } } std::unique_ptr Fusion::segment( @@ -161,10 +202,20 @@ std::unique_ptr Fusion::segment( IrCloner Fusion::copy(const Fusion* from, Fusion* to) { to->clear(); - auto ir_cloner = - IrContainer::copy(from->ir_container(), to->ir_container(), to); + IrCloner ir_cloner(to); + + // Clone from's vals in insertion order + for (auto val : from->deterministic_vals()) { + ir_cloner.clone(val); + } - // Remap cached special val pointers through the cloner + // Wire up definitions and uses on cloned vals + for (auto val : from->vals()) { + ir_cloner.clone(val)->setDefinition(ir_cloner.clone(val->definition_)); + ir_cloner.clone(val)->setUses(ir_cloner.clone(val->uses_)); + } + + // Remap cached special val pointers if (from->zero_val_) { to->zero_val_ = ir_cloner.clone(from->zero_val_); } @@ -182,11 +233,6 @@ IrCloner Fusion::copy(const Fusion* from, Fusion* to) { ir_cloner.clone(from->magic_zero_val_)->as(); } - for (auto val : from->vals()) { - ir_cloner.clone(val)->setDefinition(ir_cloner.clone(val->definition_)); - ir_cloner.clone(val)->setUses(ir_cloner.clone(val->uses_)); - } - to->inputs_ = ir_cloner.clone(from->inputs_); to->outputs_ = ir_cloner.clone(from->outputs_); for (auto inp : to->inputs_) { @@ -196,7 +242,6 @@ IrCloner Fusion::copy(const Fusion* from, Fusion* to) { out->setIsFusionOutput(true); } - // TODO: put this into ir_cloner instead for (Val* out : from->outputs_) { const AliasInfo& alias = from->io_alias_.get(out); if (alias.type == AllocationType::New) { @@ -209,14 +254,12 @@ IrCloner Fusion::copy(const Fusion* from, Fusion* to) { } to->all_tv_uses_valid_ = from->all_tv_uses_valid_; - // This should never be true on copy, but copying for completeness. to->is_during_update_uses_ = from->is_during_update_uses_; for (const auto& i : from->managed_data_) { if (i.first.has_value()) { to->managed_data_.emplace_back(i.second(ir_cloner, i.first), i.second); } else { - // Don't clone managed data if it has been reset to->managed_data_.emplace_back(i.first, i.second); } } @@ -259,9 +302,10 @@ Fusion::Fusion() : ir_container_(std::make_shared()) { ir_container_->addFusion(this); } -// Copy constructor -Fusion::Fusion(const Fusion& other) : Fusion() { +// Copy constructor -- shares the source's container +Fusion::Fusion(const Fusion& other) : ir_container_(other.ir_container_) { FUSER_PERF_SCOPE("Fusion copy"); + ir_container_->addFusion(this); Fusion::copy(&other, this); } @@ -281,6 +325,9 @@ Fusion& Fusion::operator=(const Fusion& other) { Fusion& Fusion::operator=(Fusion&& other) noexcept { FUSER_PERF_SCOPE("Fusion move assign"); + if (this == &other) { + return *this; + } clear(); swap(*this, other); return *this; From 7da05f8abd76425ee791e386161f8ba22fa5c412 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Tue, 17 Feb 2026 15:44:28 -0800 Subject: [PATCH 02/10] Per-Fusion name counters fix duplicate TV names after copy Move val/expr name counters from IrContainer to Fusion so each Fusion independently tracks name assignment. This fixes CI failures where Fusion::copy left the dest counter at N (number of cloned vals) instead of max(name)+1 when source names were non-sequential, causing newly created TVs to collide with existing names. The fix adds val_type_name_map_ and expr_name_counter_ to Fusion, and updates registerVal/registerExpr to use the Fusion-level counters. Fusion::copy syncs counters from source to dest after cloning. Fusion::swap exchanges counters. Fusion::clear resets them. --- csrc/fusion.cpp | 19 +++++++++++++++++-- csrc/fusion.h | 18 ++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/csrc/fusion.cpp b/csrc/fusion.cpp index 3bf05aa99f7..16d2a2566dc 100644 --- a/csrc/fusion.cpp +++ b/csrc/fusion.cpp @@ -158,6 +158,8 @@ void Fusion::swap(Fusion& a, Fusion& b) noexcept { std::swap(a.magic_zero_val_, b.magic_zero_val_); std::swap(a.axioms_, b.axioms_); std::swap(a.metadata_, b.metadata_); + std::swap(a.val_type_name_map_, b.val_type_name_map_); + std::swap(a.expr_name_counter_, b.expr_name_counter_); // Update Statement::ir_container_ pointers: a's old statements now belong // to b, and b's old statements now belong to a @@ -209,6 +211,16 @@ IrCloner Fusion::copy(const Fusion* from, Fusion* to) { ir_cloner.clone(val); } + // Sync per-Fusion name counters from source to dest. + // During cloning, registerVal increments the dest Fusion's counter for each + // val, then IrBuilder::clone overrides the name with setName(src->name()). + // If source names are non-sequential (e.g., {0..10, 22..27} from segmenter + // creating intermediate TVs), the dest counter ends up at N (number of vals) + // instead of max(name)+1. Copying the source's counter state ensures new + // vals created post-copy won't collide with existing names. + to->val_type_name_map_ = from->val_type_name_map_; + to->expr_name_counter_ = from->expr_name_counter_; + // Wire up definitions and uses on cloned vals for (auto val : from->vals()) { ir_cloner.clone(val)->setDefinition(ir_cloner.clone(val->definition_)); @@ -367,6 +379,9 @@ void Fusion::clear() noexcept { axioms_.reset(); metadata_.clear(); + val_type_name_map_.clear(); + expr_name_counter_ = 0; + invalidateTvsAndUses(); is_during_update_uses_ = false; @@ -971,7 +986,7 @@ void Fusion::registerVal(Val* val) { c->vals_up_.emplace_back(val); c->vals_.insert(val); c->per_fusion_vals_[this].insert(val); - val->setName(IrContainerPasskey(), c->getValName(val->vtype())); + val->setName(IrContainerPasskey(), getValName(val->vtype())); } void Fusion::registerExpr(Expr* expr) { @@ -988,7 +1003,7 @@ void Fusion::registerExpr(Expr* expr) { c->exprs_up_.emplace_back(expr); c->exprs_.insert(expr); c->per_fusion_exprs_[this].insert(expr); - expr->setName(IrContainerPasskey(), c->getExprName()); + expr->setName(IrContainerPasskey(), getExprName()); for (Val* input : expr->inputs()) { assertInContainer(input, "Input to expr is invalid, "); diff --git a/csrc/fusion.h b/csrc/fusion.h index e44817405e7..6ff140625cd 100644 --- a/csrc/fusion.h +++ b/csrc/fusion.h @@ -661,6 +661,24 @@ class NVF_API Fusion : public PolymorphicBase { std::unique_ptr> axioms_; std::unordered_map> metadata_; + + // Per-Fusion name counters. Each Fusion independently tracks name assignment + // so that cloned Fusions get matching names (T0→T0) regardless of whether + // they share an IrContainer. This is required by downstream consumers that + // use tv->name() as a map key (alias_memory, GreedyParams, etc.). + std::unordered_map val_type_name_map_; + StmtNameType expr_name_counter_ = 0; + + StmtNameType getValName(ValType vtype) { + if (val_type_name_map_.find(vtype) == val_type_name_map_.end()) { + val_type_name_map_[vtype] = 0; + } + return val_type_name_map_[vtype]++; + } + + StmtNameType getExprName() { + return expr_name_counter_++; + } }; // Template implementations for Fusion::manage() that use IrCloner From df4ab749191664be4f451f9d4195b806f825254d Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Mon, 2 Mar 2026 18:41:21 -0800 Subject: [PATCH 03/10] fix: move expr_name_counter_ sync to after expr cloning in Fusion::copy --- csrc/fusion.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/csrc/fusion.cpp b/csrc/fusion.cpp index 16d2a2566dc..f27c61a44c2 100644 --- a/csrc/fusion.cpp +++ b/csrc/fusion.cpp @@ -211,7 +211,15 @@ IrCloner Fusion::copy(const Fusion* from, Fusion* to) { ir_cloner.clone(val); } + // Wire up definitions and uses on cloned vals + for (auto val : from->vals()) { + ir_cloner.clone(val)->setDefinition(ir_cloner.clone(val->definition_)); + ir_cloner.clone(val)->setUses(ir_cloner.clone(val->uses_)); + } + // Sync per-Fusion name counters from source to dest. + // Must be AFTER all cloning (vals and exprs) so that registerVal/registerExpr + // increments during cloning do not inflate the final counter values. // During cloning, registerVal increments the dest Fusion's counter for each // val, then IrBuilder::clone overrides the name with setName(src->name()). // If source names are non-sequential (e.g., {0..10, 22..27} from segmenter @@ -221,12 +229,6 @@ IrCloner Fusion::copy(const Fusion* from, Fusion* to) { to->val_type_name_map_ = from->val_type_name_map_; to->expr_name_counter_ = from->expr_name_counter_; - // Wire up definitions and uses on cloned vals - for (auto val : from->vals()) { - ir_cloner.clone(val)->setDefinition(ir_cloner.clone(val->definition_)); - ir_cloner.clone(val)->setUses(ir_cloner.clone(val->uses_)); - } - // Remap cached special val pointers if (from->zero_val_) { to->zero_val_ = ir_cloner.clone(from->zero_val_); From fd5693e592eeb0142d8837966edfb3e9f3898125 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Mon, 2 Mar 2026 18:41:45 -0800 Subject: [PATCH 04/10] fix: enforce non-null ir_container_ invariant in Fusion::swap --- csrc/fusion.cpp | 50 ++++++++++++++++++++++--------------------------- 1 file changed, 22 insertions(+), 28 deletions(-) diff --git a/csrc/fusion.cpp b/csrc/fusion.cpp index f27c61a44c2..9e4fbd59de4 100644 --- a/csrc/fusion.cpp +++ b/csrc/fusion.cpp @@ -112,28 +112,27 @@ void Fusion::swap(Fusion& a, Fusion& b) noexcept { return; } + NVF_ERROR(a.ir_container_ != nullptr, "Fusion::swap: a has null ir_container_"); + NVF_ERROR(b.ir_container_ != nullptr, "Fusion::swap: b has null ir_container_"); + // Collect statements owned by each Fusion BEFORE swap so we can update // Statement::ir_container_ pointers afterward. std::vector a_owned_vals, b_owned_vals; std::vector a_owned_exprs, b_owned_exprs; - if (a.ir_container_) { - const auto& av = a.ir_container_->valsOwnedBy(&a); - const auto& ae = a.ir_container_->exprsOwnedBy(&a); - a_owned_vals.assign(av.begin(), av.end()); - a_owned_exprs.assign(ae.begin(), ae.end()); - } - if (b.ir_container_) { - const auto& bv = b.ir_container_->valsOwnedBy(&b); - const auto& be = b.ir_container_->exprsOwnedBy(&b); - b_owned_vals.assign(bv.begin(), bv.end()); - b_owned_exprs.assign(be.begin(), be.end()); - } + const auto& av = a.ir_container_->valsOwnedBy(&a); + const auto& ae = a.ir_container_->exprsOwnedBy(&a); + a_owned_vals.assign(av.begin(), av.end()); + a_owned_exprs.assign(ae.begin(), ae.end()); + + const auto& bv = b.ir_container_->valsOwnedBy(&b); + const auto& be = b.ir_container_->exprsOwnedBy(&b); + b_owned_vals.assign(bv.begin(), bv.end()); + b_owned_exprs.assign(be.begin(), be.end()); // Transfer Fusion registrations between containers before pointer swap. // After swap, a will own b's container and b will own a's container. - if (a.ir_container_ && b.ir_container_ && - a.ir_container_.get() != b.ir_container_.get()) { + if (a.ir_container_.get() != b.ir_container_.get()) { a.ir_container_->transferFusion(&a, &b); b.ir_container_->transferFusion(&b, &a); } @@ -176,21 +175,16 @@ void Fusion::swap(Fusion& a, Fusion& b) noexcept { expr->ir_container_ = &a; } - // Update per-Fusion tracking keys in containers - if (a.ir_container_ && b.ir_container_) { - if (a.ir_container_.get() == b.ir_container_.get()) { - // Same container: directly swap per-Fusion tracking entries - auto* c = a.ir_container_.get(); - std::swap(c->per_fusion_vals_[&a], c->per_fusion_vals_[&b]); - std::swap(c->per_fusion_exprs_[&a], c->per_fusion_exprs_[&b]); - } else { - // Different containers: rename tracking keys to match new owners - a.ir_container_->transferStatementOwnership(&b, &a); - b.ir_container_->transferStatementOwnership(&a, &b); - } - } else if (a.ir_container_) { + // Update per-Fusion tracking keys in containers. At this point, both + // a and b are guaranteed to have non-null ir_container_ (verified above). + if (a.ir_container_.get() == b.ir_container_.get()) { + // Same container: directly swap per-Fusion tracking entries + auto* c = a.ir_container_.get(); + std::swap(c->per_fusion_vals_[&a], c->per_fusion_vals_[&b]); + std::swap(c->per_fusion_exprs_[&a], c->per_fusion_exprs_[&b]); + } else { + // Different containers: rename tracking keys to match new owners a.ir_container_->transferStatementOwnership(&b, &a); - } else if (b.ir_container_) { b.ir_container_->transferStatementOwnership(&a, &b); } } From fdddb5baaffd7b4f6aa3209ec8fd2a5abe301289 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Mon, 2 Mar 2026 18:42:09 -0800 Subject: [PATCH 05/10] fix: remove noexcept from Fusion::swap --- csrc/fusion.cpp | 2 +- csrc/fusion.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/csrc/fusion.cpp b/csrc/fusion.cpp index 9e4fbd59de4..ff1cf9d5688 100644 --- a/csrc/fusion.cpp +++ b/csrc/fusion.cpp @@ -105,7 +105,7 @@ bool Fusion::sameDefinition(const Fusion& other) const { return true; } -void Fusion::swap(Fusion& a, Fusion& b) noexcept { +void Fusion::swap(Fusion& a, Fusion& b) { FUSER_PERF_SCOPE("Fusion swap"); if (&a == &b) { diff --git a/csrc/fusion.h b/csrc/fusion.h index 6ff140625cd..db8201b095a 100644 --- a/csrc/fusion.h +++ b/csrc/fusion.h @@ -187,7 +187,7 @@ class NVF_API Fusion : public PolymorphicBase { ~Fusion() override; - static void swap(Fusion& a, Fusion& b) noexcept; + static void swap(Fusion& a, Fusion& b); void clear() noexcept; From 15558bad13616015a93af8571cfff5e61e16689e Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Mon, 2 Mar 2026 18:42:19 -0800 Subject: [PATCH 06/10] style: simplify getValName to use operator[] directly --- csrc/fusion.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/csrc/fusion.h b/csrc/fusion.h index db8201b095a..13384cb0c66 100644 --- a/csrc/fusion.h +++ b/csrc/fusion.h @@ -670,9 +670,6 @@ class NVF_API Fusion : public PolymorphicBase { StmtNameType expr_name_counter_ = 0; StmtNameType getValName(ValType vtype) { - if (val_type_name_map_.find(vtype) == val_type_name_map_.end()) { - val_type_name_map_[vtype] = 0; - } return val_type_name_map_[vtype]++; } From 6d9bb48c8ec94654b2f7f23e15555371e69ce933 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Mon, 2 Mar 2026 19:09:21 -0800 Subject: [PATCH 07/10] fix: use deterministic_vals in expr wiring loop and remove noexcept from move ops --- csrc/fusion.cpp | 9 +++++---- csrc/fusion.h | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/csrc/fusion.cpp b/csrc/fusion.cpp index ff1cf9d5688..f1d0cc91571 100644 --- a/csrc/fusion.cpp +++ b/csrc/fusion.cpp @@ -205,8 +205,9 @@ IrCloner Fusion::copy(const Fusion* from, Fusion* to) { ir_cloner.clone(val); } - // Wire up definitions and uses on cloned vals - for (auto val : from->vals()) { + // Wire up definitions and uses on cloned vals in deterministic order + // to ensure exprs are inserted into exprs_up_ deterministically + for (auto val : from->deterministic_vals()) { ir_cloner.clone(val)->setDefinition(ir_cloner.clone(val->definition_)); ir_cloner.clone(val)->setUses(ir_cloner.clone(val->uses_)); } @@ -318,7 +319,7 @@ Fusion::Fusion(const Fusion& other) : ir_container_(other.ir_container_) { } // Move constructor -Fusion::Fusion(Fusion&& other) noexcept : Fusion() { +Fusion::Fusion(Fusion&& other) : Fusion() { FUSER_PERF_SCOPE("Fusion move"); swap(*this, other); } @@ -331,7 +332,7 @@ Fusion& Fusion::operator=(const Fusion& other) { return *this; } -Fusion& Fusion::operator=(Fusion&& other) noexcept { +Fusion& Fusion::operator=(Fusion&& other) { FUSER_PERF_SCOPE("Fusion move assign"); if (this == &other) { return *this; diff --git a/csrc/fusion.h b/csrc/fusion.h index 13384cb0c66..4f23130d3e9 100644 --- a/csrc/fusion.h +++ b/csrc/fusion.h @@ -180,10 +180,10 @@ class NVF_API Fusion : public PolymorphicBase { Fusion(); Fusion(const Fusion& other); - Fusion(Fusion&& other) noexcept; + Fusion(Fusion&& other); Fusion& operator=(const Fusion& other); - Fusion& operator=(Fusion&& other) noexcept; + Fusion& operator=(Fusion&& other); ~Fusion() override; From e4cf26806015bdaec8b2d32bbef363a13f1f412f Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Tue, 3 Mar 2026 11:21:57 -0800 Subject: [PATCH 08/10] Move assignment op control flow; comment. --- csrc/fusion.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/csrc/fusion.cpp b/csrc/fusion.cpp index f1d0cc91571..128fe722552 100644 --- a/csrc/fusion.cpp +++ b/csrc/fusion.cpp @@ -324,6 +324,7 @@ Fusion::Fusion(Fusion&& other) : Fusion() { swap(*this, other); } +// Copy Assignment -- shares the source's container Fusion& Fusion::operator=(const Fusion& other) { FUSER_PERF_SCOPE("Fusion copy assign"); Fusion copy(other); @@ -334,11 +335,10 @@ Fusion& Fusion::operator=(const Fusion& other) { Fusion& Fusion::operator=(Fusion&& other) { FUSER_PERF_SCOPE("Fusion move assign"); - if (this == &other) { - return *this; + if (this != &other) { + clear(); + swap(*this, other); } - clear(); - swap(*this, other); return *this; } From ba3f1a78d7614613f9a2a5deb15a12d2aef217e4 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Tue, 3 Mar 2026 12:17:50 -0800 Subject: [PATCH 09/10] Guard CpyCtor when other == this --- csrc/fusion.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/csrc/fusion.cpp b/csrc/fusion.cpp index 128fe722552..670a72e3ca3 100644 --- a/csrc/fusion.cpp +++ b/csrc/fusion.cpp @@ -327,9 +327,11 @@ Fusion::Fusion(Fusion&& other) : Fusion() { // Copy Assignment -- shares the source's container Fusion& Fusion::operator=(const Fusion& other) { FUSER_PERF_SCOPE("Fusion copy assign"); - Fusion copy(other); - clear(); - swap(*this, copy); + if (this != &other) { + Fusion copy(other); + clear(); + swap(*this, copy); + } return *this; } From c8ffe1d09e921b4d5c98afc0a07397d7b6fb994f Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Wed, 4 Mar 2026 15:27:35 -0800 Subject: [PATCH 10/10] lint --- csrc/fusion.cpp | 13 +++++++++++-- csrc/fusion.h | 9 +++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/csrc/fusion.cpp b/csrc/fusion.cpp index 670a72e3ca3..b20f1f23355 100644 --- a/csrc/fusion.cpp +++ b/csrc/fusion.cpp @@ -112,8 +112,10 @@ void Fusion::swap(Fusion& a, Fusion& b) { return; } - NVF_ERROR(a.ir_container_ != nullptr, "Fusion::swap: a has null ir_container_"); - NVF_ERROR(b.ir_container_ != nullptr, "Fusion::swap: b has null ir_container_"); + NVF_ERROR( + a.ir_container_ != nullptr, "Fusion::swap: a has null ir_container_"); + NVF_ERROR( + b.ir_container_ != nullptr, "Fusion::swap: b has null ir_container_"); // Collect statements owned by each Fusion BEFORE swap so we can update // Statement::ir_container_ pointers afterward. @@ -319,6 +321,11 @@ Fusion::Fusion(const Fusion& other) : ir_container_(other.ir_container_) { } // Move constructor +// Not marked noexcept: Fusion::swap allocates local std::vectors to collect +// statement ownership before the swap, which can throw. Since Fusions are not +// expected to be moved into containers, the performance trade-off is +// acceptable. +// NOLINTNEXTLINE(cppcoreguidelines-noexcept-move-operations) Fusion::Fusion(Fusion&& other) : Fusion() { FUSER_PERF_SCOPE("Fusion move"); swap(*this, other); @@ -335,6 +342,8 @@ Fusion& Fusion::operator=(const Fusion& other) { return *this; } +// Not marked noexcept: See move constructor above. +// NOLINTNEXTLINE(cppcoreguidelines-noexcept-move-operations) Fusion& Fusion::operator=(Fusion&& other) { FUSER_PERF_SCOPE("Fusion move assign"); if (this != &other) { diff --git a/csrc/fusion.h b/csrc/fusion.h index 4f23130d3e9..34be84be28d 100644 --- a/csrc/fusion.h +++ b/csrc/fusion.h @@ -180,9 +180,18 @@ class NVF_API Fusion : public PolymorphicBase { Fusion(); Fusion(const Fusion& other); + + // Not marked noexcept: Fusion::swap allocates local std::vectors to collect + // statement ownership before the swap, which can throw. Since Fusions are not + // expected to be moved into containers, the performance trade-off is + // acceptable. + // NOLINTNEXTLINE(cppcoreguidelines-noexcept-move-operations) Fusion(Fusion&& other); Fusion& operator=(const Fusion& other); + + // Not marked noexcept: See move constructor above. + // NOLINTNEXTLINE(cppcoreguidelines-noexcept-move-operations) Fusion& operator=(Fusion&& other); ~Fusion() override;