[multibyte] Disable multibyte load/store until interpreter is ready. - #8938
Conversation
|
It may be a bit before I can land the interpreter support, so probably good to disable for now. |
| return element.packedType == Field::i8 || element.packedType == Field::i16 || | ||
| (element.packedType == Field::NotPacked && element.type.isNumber()); | ||
| // TODO: Expand once interpreter supports additional numeric types. | ||
| return element.packedType == Field::i8; |
There was a problem hiding this comment.
Another option is to disable this feature in the fuzzer? See DISALLOWED_FEATURES_IN_V8 in scripts/fuzz_opt.py.
But I don't feel strongly if this PR is simpler for you.
There was a problem hiding this comment.
multibyte is already in DISALLOWED_FEATURES_IN_V8. I guess I'd need to put i--disable-fp16 in CONSTANT_FEATURE_OPTS to not allow it at all in binaryen?
There was a problem hiding this comment.
Yes, I think that's right - the error is in binaryen's interpreter, not v8. But maybe run the fuzzer a little locally to verify that it fixes that? It doesn't take very long to find an error.
There was a problem hiding this comment.
I.e. I would recommend running until you see an error, then apply the fix, then run that seed again with the fix to see it passes.
There was a problem hiding this comment.
On upstream/main, I see an error very quickly that doesn't look related to multibyte. I checked out my multibyte-array-expanded-types branch and it has been running for 20 minutes without an error. I'd say we just land the '--disable-multibyte' flag for now.
975a7e2 to
1f8c6a0
Compare
brendandahl
changed the title
brendandahl
changed the title
2 participants
No description provided.