Skip to content

Commit

Permalink
Make empty ids for all block types (#4502)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jonmeow
jonmeow authored Nov 7, 2024
1 parent caba03d commit cab7818
Show file tree
Hide file tree
Showing 10 changed files with 40 additions and 19 deletions.
2 changes: 1 addition & 1 deletion toolchain/check/pattern_match.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
6 changes: 4 additions & 2 deletions toolchain/check/testdata/basics/builtin_insts.carbon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
6 changes: 4 additions & 2 deletions toolchain/check/testdata/basics/no_prelude/multifile_raw_and_textual_ir.carbon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down
6 changes: 4 additions & 2 deletions toolchain/check/testdata/basics/no_prelude/multifile_raw_ir.carbon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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}
Expand Down
3 changes: 2 additions & 1 deletion toolchain/check/testdata/basics/no_prelude/raw_and_textual_ir.carbon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
3 changes: 2 additions & 1 deletion toolchain/check/testdata/basics/no_prelude/raw_ir.carbon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
13 changes: 12 additions & 1 deletion toolchain/sem_ir/block_value_store.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,18 @@ class BlockValueStore : public Yaml::Printable<BlockValueStore<IdT>> {
using ElementType = IdT::ElementType;

explicit BlockValueStore(llvm::BumpPtrAllocator& allocator)
: allocator_(&allocator) {}
: allocator_(&allocator) {
auto empty = llvm::MutableArrayRef<ElementType>();
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<ElementType> content) -> IdT {
if (content.empty()) {
return IdT::Empty;
}
return values_.Add(AllocateCopy(content));
}

Expand All @@ -50,6 +58,9 @@ class BlockValueStore : public Yaml::Printable<BlockValueStore<IdT>> {
// Adds a block or finds an existing canonical block with the given content,
// and returns an ID to reference it.
auto AddCanonical(llvm::ArrayRef<ElementType> content) -> IdT {
if (content.empty()) {
return IdT::Empty;
}
auto result = canonical_blocks_.Insert(
content, [&] { return Add(content); }, KeyContext(this));
return result.key();
Expand Down
10 changes: 10 additions & 0 deletions toolchain/sem_ir/ids.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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<TypeId> {
Expand Down Expand Up @@ -804,6 +809,10 @@ struct TypeBlockId : public IdBase, public Printable<TypeBlockId> {
// 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";
Expand All @@ -812,6 +821,7 @@ struct TypeBlockId : public IdBase, public Printable<TypeBlockId> {
};

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<ElementIndex> {
Expand Down
8 changes: 0 additions & 8 deletions toolchain/sem_ir/inst.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,8 +445,6 @@ class InstBlockStore : public BlockValueStore<InstBlockId> {

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();
Expand All @@ -455,12 +453,6 @@ class InstBlockStore : public BlockValueStore<InstBlockId> {
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<ElementType> content) -> InstBlockId {
return content.empty() ? InstBlockId::Empty : Add(content);
}

auto Set(InstBlockId block_id, llvm::ArrayRef<InstId> content) -> void {
CARBON_CHECK(block_id != InstBlockId::Unreachable);
BlockValueStore<InstBlockId>::SetContent(block_id, content);
Expand Down
2 changes: 1 addition & 1 deletion toolchain/sem_ir/yaml_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit cab7818

Please sign in to comment.