From cab7818df8ccab265aeba4279a34d43e0db6fd6d Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Thu, 7 Nov 2024 14:46:22 -0800 Subject: [PATCH] Make empty ids for all block types (#4502) Adding empty IDs to all block types. This is primarily for TypeBlockId::Empty, which allows me to disambiguate empty tuple types, which I think might yield a SemIR readability improvement (PR forthcoming). I'm splitting PRs so that the limited test impact here is clear. --- toolchain/check/pattern_match.cpp | 2 +- .../check/testdata/basics/builtin_insts.carbon | 6 ++++-- .../no_prelude/multifile_raw_and_textual_ir.carbon | 6 ++++-- .../basics/no_prelude/multifile_raw_ir.carbon | 6 ++++-- .../basics/no_prelude/raw_and_textual_ir.carbon | 3 ++- .../check/testdata/basics/no_prelude/raw_ir.carbon | 3 ++- toolchain/sem_ir/block_value_store.h | 13 ++++++++++++- toolchain/sem_ir/ids.h | 10 ++++++++++ toolchain/sem_ir/inst.h | 8 -------- toolchain/sem_ir/yaml_test.cpp | 2 +- 10 files changed, 40 insertions(+), 19 deletions(-) diff --git a/toolchain/check/pattern_match.cpp b/toolchain/check/pattern_match.cpp index d0fbf1923745..e43964def17f 100644 --- a/toolchain/check/pattern_match.cpp +++ b/toolchain/check/pattern_match.cpp @@ -126,7 +126,7 @@ auto MatchContext::DoWork(Context& context) -> SemIR::InstBlockId { while (!stack_.empty()) { EmitPatternMatch(context, stack_.pop_back_val()); } - auto block_id = context.inst_blocks().AddOrEmpty(results_); + auto block_id = context.inst_blocks().Add(results_); results_.clear(); return block_id; } diff --git a/toolchain/check/testdata/basics/builtin_insts.carbon b/toolchain/check/testdata/basics/builtin_insts.carbon index fe8ca5f9c39d..18cc407ab67c 100644 --- a/toolchain/check/testdata/basics/builtin_insts.carbon +++ b/toolchain/check/testdata/basics/builtin_insts.carbon @@ -23,12 +23,14 @@ // CHECK:STDOUT: classes: {} // CHECK:STDOUT: generics: {} // CHECK:STDOUT: specifics: {} -// CHECK:STDOUT: struct_type_fields: {} +// CHECK:STDOUT: struct_type_fields: +// CHECK:STDOUT: type_block0: {} // CHECK:STDOUT: types: // CHECK:STDOUT: typeTypeType: {kind: copy, type: typeTypeType} // CHECK:STDOUT: typeError: {kind: copy, type: typeError} // CHECK:STDOUT: 'type(instNamespaceType)': {kind: copy, type: type(instNamespaceType)} -// CHECK:STDOUT: type_blocks: {} +// CHECK:STDOUT: type_blocks: +// CHECK:STDOUT: type_block0: {} // CHECK:STDOUT: insts: // CHECK:STDOUT: instTypeType: {kind: BuiltinInst, arg0: TypeType, type: typeTypeType} // CHECK:STDOUT: instError: {kind: BuiltinInst, arg0: Error, type: typeError} diff --git a/toolchain/check/testdata/basics/no_prelude/multifile_raw_and_textual_ir.carbon b/toolchain/check/testdata/basics/no_prelude/multifile_raw_and_textual_ir.carbon index 197d9f648c00..0b39231a2dc8 100644 --- a/toolchain/check/testdata/basics/no_prelude/multifile_raw_and_textual_ir.carbon +++ b/toolchain/check/testdata/basics/no_prelude/multifile_raw_and_textual_ir.carbon @@ -40,7 +40,8 @@ fn B() { // CHECK:STDOUT: classes: {} // CHECK:STDOUT: generics: {} // CHECK:STDOUT: specifics: {} -// CHECK:STDOUT: struct_type_fields: {} +// CHECK:STDOUT: struct_type_fields: +// CHECK:STDOUT: type_block0: {} // CHECK:STDOUT: types: // CHECK:STDOUT: typeTypeType: {kind: copy, type: typeTypeType} // CHECK:STDOUT: typeError: {kind: copy, type: typeError} @@ -116,7 +117,8 @@ fn B() { // CHECK:STDOUT: classes: {} // CHECK:STDOUT: generics: {} // CHECK:STDOUT: specifics: {} -// CHECK:STDOUT: struct_type_fields: {} +// CHECK:STDOUT: struct_type_fields: +// CHECK:STDOUT: type_block0: {} // CHECK:STDOUT: types: // CHECK:STDOUT: typeTypeType: {kind: copy, type: typeTypeType} // CHECK:STDOUT: typeError: {kind: copy, type: typeError} diff --git a/toolchain/check/testdata/basics/no_prelude/multifile_raw_ir.carbon b/toolchain/check/testdata/basics/no_prelude/multifile_raw_ir.carbon index 1205d7423a62..8401e71df324 100644 --- a/toolchain/check/testdata/basics/no_prelude/multifile_raw_ir.carbon +++ b/toolchain/check/testdata/basics/no_prelude/multifile_raw_ir.carbon @@ -40,7 +40,8 @@ fn B() { // CHECK:STDOUT: classes: {} // CHECK:STDOUT: generics: {} // CHECK:STDOUT: specifics: {} -// CHECK:STDOUT: struct_type_fields: {} +// CHECK:STDOUT: struct_type_fields: +// CHECK:STDOUT: type_block0: {} // CHECK:STDOUT: types: // CHECK:STDOUT: typeTypeType: {kind: copy, type: typeTypeType} // CHECK:STDOUT: typeError: {kind: copy, type: typeError} @@ -95,7 +96,8 @@ fn B() { // CHECK:STDOUT: classes: {} // CHECK:STDOUT: generics: {} // CHECK:STDOUT: specifics: {} -// CHECK:STDOUT: struct_type_fields: {} +// CHECK:STDOUT: struct_type_fields: +// CHECK:STDOUT: type_block0: {} // CHECK:STDOUT: types: // CHECK:STDOUT: typeTypeType: {kind: copy, type: typeTypeType} // CHECK:STDOUT: typeError: {kind: copy, type: typeError} diff --git a/toolchain/check/testdata/basics/no_prelude/raw_and_textual_ir.carbon b/toolchain/check/testdata/basics/no_prelude/raw_and_textual_ir.carbon index 21cf677df4e2..3256a2ddab34 100644 --- a/toolchain/check/testdata/basics/no_prelude/raw_and_textual_ir.carbon +++ b/toolchain/check/testdata/basics/no_prelude/raw_and_textual_ir.carbon @@ -31,7 +31,8 @@ fn Foo(n: ()) -> ((), ()) { // CHECK:STDOUT: classes: {} // CHECK:STDOUT: generics: {} // CHECK:STDOUT: specifics: {} -// CHECK:STDOUT: struct_type_fields: {} +// CHECK:STDOUT: struct_type_fields: +// CHECK:STDOUT: type_block0: {} // CHECK:STDOUT: types: // CHECK:STDOUT: typeTypeType: {kind: copy, type: typeTypeType} // CHECK:STDOUT: typeError: {kind: copy, type: typeError} diff --git a/toolchain/check/testdata/basics/no_prelude/raw_ir.carbon b/toolchain/check/testdata/basics/no_prelude/raw_ir.carbon index b08453e7564a..56eac09fd279 100644 --- a/toolchain/check/testdata/basics/no_prelude/raw_ir.carbon +++ b/toolchain/check/testdata/basics/no_prelude/raw_ir.carbon @@ -34,7 +34,8 @@ fn Foo[T:! type](n: T) -> (T, ()) { // CHECK:STDOUT: generic0: {decl: inst+24, bindings: block11} // CHECK:STDOUT: specifics: // CHECK:STDOUT: specific0: {generic: generic0, args: block13} -// CHECK:STDOUT: struct_type_fields: {} +// CHECK:STDOUT: struct_type_fields: +// CHECK:STDOUT: type_block0: {} // CHECK:STDOUT: types: // CHECK:STDOUT: typeTypeType: {kind: copy, type: typeTypeType} // CHECK:STDOUT: typeError: {kind: copy, type: typeError} diff --git a/toolchain/sem_ir/block_value_store.h b/toolchain/sem_ir/block_value_store.h index fbd7e6f63699..6149d7affe4a 100644 --- a/toolchain/sem_ir/block_value_store.h +++ b/toolchain/sem_ir/block_value_store.h @@ -30,10 +30,18 @@ class BlockValueStore : public Yaml::Printable> { using ElementType = IdT::ElementType; explicit BlockValueStore(llvm::BumpPtrAllocator& allocator) - : allocator_(&allocator) {} + : allocator_(&allocator) { + auto empty = llvm::MutableArrayRef(); + auto empty_val = canonical_blocks_.Insert( + empty, [&] { return values_.Add(empty); }, KeyContext(this)); + CARBON_CHECK(empty_val.key() == IdT::Empty); + } // Adds a block with the given content, returning an ID to reference it. auto Add(llvm::ArrayRef content) -> IdT { + if (content.empty()) { + return IdT::Empty; + } return values_.Add(AllocateCopy(content)); } @@ -50,6 +58,9 @@ class BlockValueStore : public Yaml::Printable> { // Adds a block or finds an existing canonical block with the given content, // and returns an ID to reference it. auto AddCanonical(llvm::ArrayRef content) -> IdT { + if (content.empty()) { + return IdT::Empty; + } auto result = canonical_blocks_.Insert( content, [&] { return Add(content); }, KeyContext(this)); return result.key(); diff --git a/toolchain/sem_ir/ids.h b/toolchain/sem_ir/ids.h index fbd95f0e784d..0c91576246a8 100644 --- a/toolchain/sem_ir/ids.h +++ b/toolchain/sem_ir/ids.h @@ -732,6 +732,10 @@ struct StructTypeFieldsId : public IdBase, // An explicitly invalid ID. static const StructTypeFieldsId Invalid; + // The canonical empty block, reused to avoid allocating empty vectors. Always + // the 0-index block. + static const StructTypeFieldsId Empty; + using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { out << "type_block"; @@ -741,6 +745,7 @@ struct StructTypeFieldsId : public IdBase, constexpr StructTypeFieldsId StructTypeFieldsId::Invalid = StructTypeFieldsId(InvalidIndex); +constexpr StructTypeFieldsId StructTypeFieldsId::Empty = StructTypeFieldsId(0); // The ID of a type. struct TypeId : public IdBase, public Printable { @@ -804,6 +809,10 @@ struct TypeBlockId : public IdBase, public Printable { // An explicitly invalid ID. static const TypeBlockId Invalid; + // The canonical empty block, reused to avoid allocating empty vectors. Always + // the 0-index block. + static const TypeBlockId Empty; + using IdBase::IdBase; auto Print(llvm::raw_ostream& out) const -> void { out << "type_block"; @@ -812,6 +821,7 @@ struct TypeBlockId : public IdBase, public Printable { }; constexpr TypeBlockId TypeBlockId::Invalid = TypeBlockId(InvalidIndex); +constexpr TypeBlockId TypeBlockId::Empty = TypeBlockId(0); // An index for element access, for structs, tuples, and classes. struct ElementIndex : public IndexBase, public Printable { diff --git a/toolchain/sem_ir/inst.h b/toolchain/sem_ir/inst.h index e75cbbef5c3f..5e2c11f3e6a1 100644 --- a/toolchain/sem_ir/inst.h +++ b/toolchain/sem_ir/inst.h @@ -445,8 +445,6 @@ class InstBlockStore : public BlockValueStore { explicit InstBlockStore(llvm::BumpPtrAllocator& allocator) : BaseType(allocator) { - auto empty_id = AddCanonical({}); - CARBON_CHECK(empty_id == InstBlockId::Empty); auto exports_id = AddDefaultValue(); CARBON_CHECK(exports_id == InstBlockId::Exports); auto import_refs_id = AddDefaultValue(); @@ -455,12 +453,6 @@ class InstBlockStore : public BlockValueStore { CARBON_CHECK(global_init_id == InstBlockId::GlobalInit); } - // Adds a block with the given content, returning an ID to reference it. - // Returns Empty rather than creating a unique ID if the block is empty. - auto AddOrEmpty(llvm::ArrayRef content) -> InstBlockId { - return content.empty() ? InstBlockId::Empty : Add(content); - } - auto Set(InstBlockId block_id, llvm::ArrayRef content) -> void { CARBON_CHECK(block_id != InstBlockId::Unreachable); BlockValueStore::SetContent(block_id, content); diff --git a/toolchain/sem_ir/yaml_test.cpp b/toolchain/sem_ir/yaml_test.cpp index 6d9126542be9..3d3df143d1f8 100644 --- a/toolchain/sem_ir/yaml_test.cpp +++ b/toolchain/sem_ir/yaml_test.cpp @@ -63,7 +63,7 @@ TEST(SemIRTest, YAML) { Pair("classes", Yaml::Mapping(SizeIs(0))), Pair("generics", Yaml::Mapping(SizeIs(0))), Pair("specifics", Yaml::Mapping(SizeIs(0))), - Pair("struct_type_fields", Yaml::Mapping(SizeIs(0))), + Pair("struct_type_fields", Yaml::Mapping(SizeIs(1))), Pair("types", Yaml::Mapping(Each(type_builtin))), Pair("type_blocks", Yaml::Mapping(SizeIs(Ge(1)))), Pair("insts",