Fix RTTs for RTT-less instructions#4294
Conversation
| } | ||
| Literal actualRtt; | ||
| if (original.isFunction()) { | ||
| // Function references always have the canonical rTTs of the functions |
There was a problem hiding this comment.
| // Function references always have the canonical rTTs of the functions | |
| // Function references always have the canonical RTTs of the functions |
| } else if (cast.getSuccess()) { | ||
| return Literal(int32_t(1)); | ||
| } else { | ||
| return Literal(int32_t(0)); |
There was a problem hiding this comment.
| } else if (cast.getSuccess()) { | |
| return Literal(int32_t(1)); | |
| } else { | |
| return Literal(int32_t(0)); | |
| } else { | |
| return Literal(int32_t(cast.getSuccess() ? 1 : 0)); | |
| } |
There was a problem hiding this comment.
Will use the similar bool(getSuccess()).
|
Added tests building on top of #4301. Here's the diff specific to this PR: https://github.com/WebAssembly/binaryen/pull/4294/files/4a53df9c4b83bff7c8f028814dcb1af5ab90391a..21e9c5bb5d620e79bafbed10d590dd04e33bc098 |
|
Code looks good! For the tests, should I review a diff of them after the other PR lands? |
|
Oops, I see you wrote that in a comment... |
| ) | ||
| ) | ||
|
|
||
| (func $make-struct-canon (result (ref $struct)) |
There was a problem hiding this comment.
this makes a sub-struct - perhaps the name should reflect that?
There was a problem hiding this comment.
Thanks, made that change when I moved this test to #4301.
| (call $log | ||
| (ref.test_static $sub-struct | ||
| (call $make-struct-canon) | ||
| ) |
There was a problem hiding this comment.
(renaming that function would make it more obvious why this cast works)
Allocation and cast instructions without explicit RTTs should use the canonical RTTs for the given types. Furthermore, the RTTs for nominal types should reflect the static type hierarchy. Previously, however, we implemented allocations and casts without RTTs using an alternative system that only used static types rather than RTT values. This alternative system would work fine in a world without first-class RTTs, but it did not properly allow mixing instructions that use RTTs and instructions that do not use RTTs as intended by the M4 GC spec. This PR fixes the issue by using canonical RTTs where appropriate and cleans up the relevant casting code using std::variant.
Allocation and cast instructions without explicit RTTs should use the canonical
RTTs for the given types. Furthermore, the RTTs for nominal types should reflect
the static type hierarchy. Previously, however, we implemented allocations and
casts without RTTs using an alternative system that only used static types
rather than RTT values. This alternative system would work fine in a world
without first-class RTTs, but it did not properly allow mixing instructions that
use RTTs and instructions that do not use RTTs as intended by the M4 GC spec.
This PR fixes the issue by using canonical RTTs where appropriate and cleans up
the relevant casting code using std::variant.
The code in this PR should be complete, but it will need tests.