From 4e086a661555f93153652dd92d017a4fe2d43fbc Mon Sep 17 00:00:00 2001 From: Geoff Romer Date: Thu, 18 Jun 2026 17:22:05 -0700 Subject: [PATCH] Include bundle operands in operand refinement (#7391) Also some Bundle API tweaks: - Remove support for non-canonical bundle IDs. Bundles don't have a unique identity, so non-canonical bundle IDs would bloat the SemIR for no benefit. - Adjust the conversions between raw and typed bundle IDs to not be templated. This makes the conversions easier to access in a debugger. --- toolchain/check/action.cpp | 88 ++++++++++++++++++++----------- toolchain/check/pattern_match.cpp | 7 +-- toolchain/sem_ir/bundle.h | 18 ++----- toolchain/sem_ir/ids.h | 24 ++++----- 4 files changed, 77 insertions(+), 60 deletions(-) diff --git a/toolchain/check/action.cpp b/toolchain/check/action.cpp index b9c6a1ab703b..5494e2835a61 100644 --- a/toolchain/check/action.cpp +++ b/toolchain/check/action.cpp @@ -222,39 +222,67 @@ static auto AddDependentActionSpliceImpl(Context& context, // produces an argument that has the template-dependent parts replaced with // their concrete values, so that the action doesn't need to know which specific // it is operating on. -static auto RefineOperand(Context& context, SemIR::LocId loc_id, - SemIR::IdAndKind arg) -> int32_t { - if (auto inst_id = arg.TryAs()) { - auto inst = context.insts().Get(*inst_id); - if (inst.Is()) { - // The argument will evaluate to the spliced instruction, which is already - // refined. - return arg.value(); - } +// +// This is the default case, for ID kinds that can't be refined. +template + requires SemIR::Internal::IsIdKindType +static auto RefineTypedOperand(Context& /*context*/, SemIR::LocId /*loc_id*/, + IdT id) -> IdT { + return id; +} - // If the type of the action argument is dependent, refine to an instruction - // with a concrete type. - if (OperandDependence(context, inst.type_id()) == - SemIR::ConstantDependence::Template) { - auto type_inst_id = context.types().GetTypeInstId(inst.type_id()); - inst_id = AddDependentActionSpliceImpl( - context, - SemIR::LocIdAndInst( - loc_id, - SemIR::RefineTypeAction{.type_id = GetSingletonType( - context, SemIR::InstType::TypeInstId), - .inst_id = *inst_id, - .inst_type_inst_id = type_inst_id}), - type_inst_id); - } - - // TODO: Handle the case where the constant value of the instruction is - // template-dependent. - - return inst_id->index; +static auto RefineTypedOperand(Context& context, SemIR::LocId loc_id, + SemIR::MetaInstId inst_id) -> SemIR::MetaInstId { + auto inst = context.insts().Get(inst_id); + if (inst.Is()) { + // The argument will evaluate to the spliced instruction, which is already + // refined. + return inst_id; } - return arg.value(); + // If the type of the action argument is dependent, refine to an instruction + // with a concrete type. + if (OperandDependence(context, inst.type_id()) == + SemIR::ConstantDependence::Template) { + auto type_inst_id = context.types().GetTypeInstId(inst.type_id()); + inst_id = AddDependentActionSpliceImpl( + context, + SemIR::LocIdAndInst( + loc_id, + SemIR::RefineTypeAction{.type_id = GetSingletonType( + context, SemIR::InstType::TypeInstId), + .inst_id = inst_id, + .inst_type_inst_id = type_inst_id}), + type_inst_id); + } + + // TODO: Handle the case where the constant value of the instruction is + // template-dependent. + + return inst_id; +} + +template +static auto RefineTypedOperand(Context& context, SemIR::LocId loc_id, + SemIR::BundleId bundle_id) + -> SemIR::BundleId { + auto bundle_tuple = context.bundles().GetAsTuple(bundle_id); + BundleT refined_bundle = std::apply( + [&](auto... bundle_fields) { + // This can't actually recurse, because bundles can't contain bundle + // IDs. + return BundleT{RefineTypedOperand(context, loc_id, bundle_fields)...}; + }, + bundle_tuple); + return context.bundles().AddCanonical(refined_bundle); +} + +// Dynamically dispatched wrapper for RefineTypedOperand. +static auto RefineOperand(Context& context, SemIR::LocId loc_id, + SemIR::IdAndKind arg) -> int32_t { + return arg.Dispatch([&](auto id) { + return SemIR::ToRaw(RefineTypedOperand(context, loc_id, id)); + }); } // Refine the operands of an action, ensuring that they will refer to concrete diff --git a/toolchain/check/pattern_match.cpp b/toolchain/check/pattern_match.cpp index f3bc8d71a7a0..fcb750c20f45 100644 --- a/toolchain/check/pattern_match.cpp +++ b/toolchain/check/pattern_match.cpp @@ -833,9 +833,10 @@ auto MatchContext::DoPreWork(State state, SemIR::SpliceInst splice, context_.types().GetTypeInstId(splice.type_id), {.type_id = SemIR::InstType::TypeId, .args_id = - context_.bundles().Add( - {.pattern_id = entry.pattern_id, - .parent_index = callee_state->index.Allocate()})}); + context_.bundles() + .AddCanonical( + {.pattern_id = entry.pattern_id, + .parent_index = callee_state->index.Allocate()})}); callee_state->PushCallParamPattern( context_, SemIR::LocId(entry.pattern_id), entry.pattern_id, specific_id_stack_.back()); diff --git a/toolchain/sem_ir/bundle.h b/toolchain/sem_ir/bundle.h index 4dda923cd015..eb07fa0f8be5 100644 --- a/toolchain/sem_ir/bundle.h +++ b/toolchain/sem_ir/bundle.h @@ -43,8 +43,9 @@ namespace Carbon::SemIR { // // A bundle type like `Args` must be an aggregate, and its fields must all be // ID types, i.e. types listed in the definition of `IdKind`. `BundleId` -// itself must also be added to that list, although bundles should generally not -// have bundle IDs as members. +// itself must also be added to that list, although bundles must not have +// bundle IDs as members (bundles are an optimization of a flat argument list, +// not a recursive data structure). // // Unlike insts, bundles do not record their own kind and the `BundleStore` // is not guaranteed to record it either. Instead, that information is tracked @@ -71,12 +72,6 @@ class BundleStore { explicit BundleStore(llvm::BumpPtrAllocator& allocator, CheckIRId tag_id) : store_(allocator, tag_id, 0), bundle_kind_cache_(store_.GetIdTag()) {} - // Adds a new bundle to the store, and returns its ID. - template - auto Add(const BundleT& bundle) -> BundleId { - return BundleId{store_.Add(BundleToArray(bundle))}; - } - // Returns the canonical ID of the given bundle, allocating a new one if // it does not already exist. template @@ -84,13 +79,6 @@ class BundleStore { return BundleId{store_.AddCanonical(BundleToArray(bundle))}; } - // Returns the canonical ID of the bundle specified by `bundle_id`, allocating - // a new canonical ID if none exists already. - template - auto MakeCanonical(BundleId bundle_id) -> BundleId { - return BundleId{store_.MakeCanonical(bundle_id.index)}; - } - // Returns the bundle with the given ID. template auto Get(BundleId bundle_id) const -> BundleT { diff --git a/toolchain/sem_ir/ids.h b/toolchain/sem_ir/ids.h index 63018f35724e..9a6c955e69d1 100644 --- a/toolchain/sem_ir/ids.h +++ b/toolchain/sem_ir/ids.h @@ -1010,26 +1010,26 @@ struct RequireImplsBlockId : public IdBase { inline constexpr RequireImplsBlockId RequireImplsBlockId::Empty = RequireImplsBlockId(0); +// The ID of a bundle of arguments with an unspecified type. +struct RawBundleId : public IdBase { + static constexpr llvm::StringLiteral Label = "bundle"; + + using IdBase::IdBase; +}; + // The ID of a bundle of arguments with type `BundleT`. template struct BundleId : public IdBase> { static constexpr llvm::StringLiteral Label = "bundle"; using IdBase>::IdBase; -}; -// The ID of a bundle of arguments with an unspecified type. -struct RawBundleId : public IdBase { - static constexpr llvm::StringLiteral Label = "bundle"; + explicit BundleId(RawBundleId raw_id) + : IdBase>(raw_id.index) {} - template - explicit(false) RawBundleId(BundleId bundle_id) - : IdBase(bundle_id.index) {} - using IdBase::IdBase; - - template - explicit operator BundleId() const { - return BundleId(index); + // NOLINTNEXTLINE(google-explicit-constructor) + explicit(false) operator RawBundleId() const { + return RawBundleId(this->index); } };