[Wasm GC] Optimize casts to bottom types #5484
Conversation
|
|
||
| auto refIsHeapSubType = HeapType::isSubType(refHeapType, castHeapType); | ||
| auto castIsHeapSubType = HeapType::isSubType(castHeapType, refHeapType); | ||
| bool heapTypesCompatible = refIsHeapSubType || castIsHeapSubType; |
There was a problem hiding this comment.
This smaller diff would accomplish the same thing.
| bool heapTypesCompatible = refIsHeapSubType || castIsHeapSubType; | |
| // The dynamic heap type of the reference might be a subtype of the cast type only if the types are related and the cast type is instantiable. | |
| bool heapTypesCompatible = (refIsHeapSubType || castIsHeapSubType) && !castHeapType.isBottom(); |
There was a problem hiding this comment.
What does "instantiable" mean? A null is a real value, so I guess you mean "not created by struct.new" or such?
But even so I'm not sure how to read the English here. Why can't the cast type be a bottom type for the types to be compatible? A null input value would be compatible with a null cast type, wouldn't it?
There was a problem hiding this comment.
I do see how the logic is repeated, though... but I'm not sure how to explain a merge of the code...
There was a problem hiding this comment.
I pushed a commit that unifies the code, but now the comment is twice as long 🤔
There was a problem hiding this comment.
It’s not the reference type but rather the bottom heap type that is not instantiable, i.e. not able to be instantiated.
“compatible” here means it’s possible for the dynamic heap type of the referenced object to be a subtype of the cast target heap. If the referenced heap type is bottom, the referenced object cannot exist so the heap types cannot be compatible.
There was a problem hiding this comment.
I guess you're special-casing null in the last paragraph? In my mind a null value is compatible with a null cast target. Why is that wrong?
That is, no instantiated object exists (as you said) but a null is still a real value, and it has a heap type, and that heap type is compatible with the cast type's heap type. So the heap types are compatible.
There was a problem hiding this comment.
FWIW I'm fine with the way you've written it as well, where special casing bottom is done separately from the computation of "compatibility."
| ;; we dismiss the possibility of a trap and just return a null. Or, if the | ||
| ;; input is non-nullable, we will trap in either mode. |
There was a problem hiding this comment.
It would be clearer to comment on the two casts below separately. The first is more similar to the two casts above than to the last cast.
tlively
left a comment
tlively
left a comment
There was a problem hiding this comment.
I think we can simplify the comment while still making the situation clear.
Co-authored-by: Thomas Lively <tlively@google.com>
A cast to a non-nullable null (an impossible type) must trap.
In traps-never-happen mode, a cast that either returns a null or traps will
definitely return a null.
Followup to #5461 which emits casts to bottom types.