Initial support for exact heap types by tlively · Pull Request #7396 · WebAssembly/binaryen

tlively · 2025-03-25T03:48:13Z

The custom descriptors proposal has moved exactness from reference types
to defined (but not abstract) heap types. Since we only use a bit in the
heap type representation to represent sharedness for abstract heap
types, we can conveniently reuse the same bit to represent exactness for
heap types.

Implement basic support for representing exact heap types and taking
them into account in canonicalization. Also ensure that other operations
like getting the rec group of a heap type or looking up its structure
work properly on exact heap types.

The length macro used in a type test in type-builder.cpp was causing extremely long compile times in some compilers. Use a lambda instead to fix it. This makes the error messages less useful when a test fails, but under normal circumstances the test should not be failing, so this is a good trade off. Fixes #7383.

The custom descriptors proposal has moved exactness from reference types to defined (but not abstract) heap types. Since we only use a bit in the heap type representation to represent sharedness for abstract heap types, we can conveniently reuse the same bit to represent exactness for heap types. Implement basic support for representing exact heap types and taking them into account in canonicalization. Also ensure that other operations like getting the rec group of a heap type or looking up its structure work properly on exact heap types.

kripken · 2025-03-25T16:57:18Z

-  // Bits 0-2 are used by the Type representation, so need to be left free.
-  // Bit 3 determines whether the basic heap type is shared (1) or unshared (0).
+  // Bits 0-1 are used by the Type representation, so need to be left free.
+  // Bit 2 determines whether basic heap type is shared (1) or unshared (0).


Suggested change

// Bit 2 determines whether basic heap type is shared (1) or unshared (0).

// Bit 2 determines whether a basic heap type is shared (1) or unshared (0).

kripken · 2025-03-25T16:59:36Z

 HeapTypeInfo* getHeapTypeInfo(HeapType ht) {
  assert(!ht.isBasic());
-  return (HeapTypeInfo*)ht.getID();
+  return (HeapTypeInfo*)(ht.with(Inexact).getID());


Why is this correct?

This clears the exact bit in the ID, recovering the value of the pointer to the underlying HeapTypeInfo.

Then I'm surprised we don't need to clear out the other 2 reserved bits? In what way is bit 2 different than 0-1?

Bits 0-1 are used in the Type representation and the RecGroup representation, but are always zero for HeapType.

I see, thanks.

Perhaps we can add a helper for zeroing out the relevant bits? That would be clearer than with(Inexact) and it would also be much easier to update in the future if we add more special bits.

See the rawID commit for this.

kripken · 2025-03-25T17:00:31Z

It is convenient to reuse the same bit, but still, maybe it makes sense to back out the old exactness? I just imagine that if I run into some exactness-related bug, debugging it will be much harder if the issue could be one of two exactness representations.

tlively · 2025-03-25T17:16:40Z

Yes, I will definitely back out the exactness on reference types, but I wanted to wait until we had a chance to validate that there are no huge issues with putting the exactness on the heap types instead.

tlively · 2025-03-26T01:51:35Z

The problem with the Emscripten tests is that the 8-byte alignment provided by Emscripten's allocator means that the addresses of allocated HeapTypeInfos might use bit 3 and conflict with our use of it to represent exactness. To fix this problem I could either increase the alignment of all HeapTypeInfo allocations or I could back out exact reference types to allow us to use bit 2 to represent heap type exactness. On the one hand, increasing the allocation alignment would be wasted work since we plan to back out exact reference types anyway, but on the other hand we may need to increase the allocation alignment anyway if we need to use more bits in the future.

We decided that Custom Descriptors should introduce exact heap types rather than exact reference types. Although these new features are very similar, the APIs we need to change for them are completely different. One option would have been to keep the existing exact reference type implementation while additionally implementing exact heap types, but there are not enough free bits in the type implementation to have both at once without increasing the alignment of HeapTypeInfo allocations. Portably increasing the alignment is annoying enough that it's easier to just eagerly remove exact reference types to free up the bit for use with exact heap types. Fully or partially revert the following PRs: - #7371 - #7365 - #7360 - #7357 - #7356 - #7355 - #7354 - #7353 - #7347 - #7342 - #7328 Keep the new `.with(...)` Type APIs and the relevant parts of the type relations gtest that were introduced as part of the reverted work.

Now that we aren't supporting exact reference types, we no longer need to leave bit 2 free for use by the Type representation. Shift the basic HeapType representations down to start at bit 2 instead of bit 3.

kripken · 2025-03-26T19:02:04Z

+  }
+
+  constexpr TypeID getID() const { return id; }
+  constexpr TypeID getRawID() const {


Please add a comment explaining the difference.

And, in particular - when would getID() still be used?

For example when hashing a type or in the C API to convert the type to an integer.

Added the comment.

kripken · 2025-03-26T19:05:02Z

+        ht = ht.with(Inexact);
        if (auto it = canonicalized.find(ht); it != canonicalized.end()) {
-          *type = Type(it->second, type->getNullability());
+          *type = Type(it->second.with(exact), type->getNullability());


But if exactness is part of the heap type, why do we want this behavior?

This is replacing non-canonical heap types with canonical heap types. If $t' is the canonical version of $t, we need to update uses of (exact $t) to be (exact $t') instead. The exact type will not appear in the canonicalized map because we are canonicalizing type definitions represented by the inexact defined types. (exact $t) and $t refer to the same type definition, so it would be redundant to store the exact version in the map.

I see, thanks. Why does Shared not need to be handled similarly here? Because for non-basic types we store it on HeapTypeInfo?

Right. Sharedness is actually part of the type definition, unlike exactness.

kripken · 2025-03-26T21:01:33Z

 HeapTypeInfo* getHeapTypeInfo(HeapType ht) {
  assert(!ht.isBasic());
-  return (HeapTypeInfo*)ht.getID();
+  return (HeapTypeInfo*)(ht.getRawID());


I don't feel strongly but getRawID could maybe be getPointer.

What do you think about getDefinitionID or getBaseID?

Hmm, no real preference between those. Current name is fine.

kripken · 2025-03-26T21:07:35Z

+        ht = ht.with(Inexact);
        if (auto it = canonicalized.find(ht); it != canonicalized.end()) {
-          *type = Type(it->second, type->getNullability());
+          *type = Type(it->second.with(exact), type->getNullability());


I see, thanks. Why does Shared not need to be handled similarly here? Because for non-basic types we store it on HeapTypeInfo?

Revert the following PRs that introduced exactness on HeapType: - #7446 - #7444 - #7432 - #7412 - #7396 Keep only the changes to wasm-type-printing.cpp to fix the assertions that subclasses of TypeNameGeneratorBase correctly override getNames. Although putting exactness on heap types makes the most sense in the Custom Descriptors spec, it is not a great fit for how HeapType is used in Binaryen. In almost all cases, HeapType is used to represent heap type definitions rather than the dynamic types of values. Letting HeapType represent exact heap types is not useful in those cases and opens up a new class of possible bugs. To avoid these bugs, we had been introducing a new HeapTypeDef type to represent heap type definitions, but nearly all uses of HeapType would have had to have been replaced with HeapTypeDef. Rather than introduce HeapTypeDef as a third type alongside Type and HeapType, it will be simpler to continue using HeapType to represent heap type definitions and Type to represent the dynamic types of values, including their exactness. While exactness will syntactically be part of the heap type, it will be part of the `Type` in Binaryen IR. A follow-on PR will restore the original work on exact references, and PRs following that one will update the encoding, parsing, and printing of exact references to match the current spec.

tlively added 2 commits March 24, 2025 19:27

tlively requested a review from kripken March 25, 2025 03:48

Base automatically changed from fix-type-builder-gtest-compile-time to main March 25, 2025 03:48

kripken reviewed Mar 25, 2025

View reviewed changes

tlively added 2 commits March 25, 2025 10:19

Merge branch 'main' into exact-heap-types

1817627

fix comment

fd709ed

tlively added 10 commits March 25, 2025 19:37

Use fewer bits for BasicHeapType

a475c44

Now that we aren't supporting exact reference types, we no longer need to leave bit 2 free for use by the Type representation. Shift the basic HeapType representations down to start at bit 2 instead of bit 3.

Merge branch 'main' into remove-exact-references

35aa881

Merge branch 'main' into exact-heap-types

e23dc6a

Merge branch 'remove-exact-references' into no-exact-reftype-repr

91155ad

Merge branch 'main' into exact-heap-types

c3a12b2

Merge branch 'main' into no-exact-reftype-repr

5e343e0

Merge branch 'no-exact-reftype-repr' into exact-heap-types

6382e0d

rawID

dcf0a17

Merge branch 'main' into exact-heap-types

d48547c

kripken reviewed Mar 26, 2025

View reviewed changes

rawID comment

c594874

kripken approved these changes Mar 26, 2025

View reviewed changes

tlively merged commit 5f6ba29 into main Mar 27, 2025

tlively deleted the exact-heap-types branch March 27, 2025 00:20

tlively mentioned this pull request Apr 15, 2025

Revert exact HeapTypes #7496

Merged

	// Bit 2 determines whether basic heap type is shared (1) or unshared (0).
	// Bit 2 determines whether a basic heap type is shared (1) or unshared (0).

Uh oh!

Conversation

tlively commented Mar 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kripken commented Mar 25, 2025

Uh oh!

tlively commented Mar 25, 2025

Uh oh!

tlively commented Mar 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants