Skip to content

Conversation

kripken
Copy link
Member

@kripken kripken commented Sep 4, 2025

No description provided.

@kripken kripken requested a review from tlively September 4, 2025 16:59
Copy link
Member

@tlively tlively left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like we don't do skipNonNullCast on a ref.cast_desc descriptor, either, although we do already have trapOnNull. We don't have either for br_on_cast_desc{_fail}.

@kripken
Copy link
Member Author

kripken commented Sep 4, 2025

It looks like we don't do skipNonNullCast on a ref.cast_desc descriptor, either, although we do already have trapOnNull.

Is that valid? ref.cast_desc can succeed on a null descriptor, I think, if it is a nullable cast and the input is null?

@tlively
Copy link
Member

tlively commented Sep 4, 2025

No, it always traps on a null descriptor no matter what.

@kripken
Copy link
Member Author

kripken commented Sep 4, 2025

I don't see that in wasm-interpreter.h. Am I missing it, or is that a bug?

@tlively
Copy link
Member

tlively commented Sep 4, 2025

It's there in doDescCast:

    auto expected = desc.getSingleValue().getGCData();
    if (!expected) {
      trap("null descriptor");
    }

@kripken
Copy link
Member Author

kripken commented Sep 4, 2025

Sorry, I somehow read that twice and missed it...

@kripken
Copy link
Member Author

kripken commented Sep 4, 2025

Ok, added those too.

@kripken kripken changed the title [Custom Descriptors] Optimize ref.get_desc in OptimizeInstructions [Custom Descriptors] OptimizeInstructions: Optimize trapping in descriptor operations Sep 4, 2025
@kripken kripken merged commit 56ba39b into WebAssembly:main Sep 4, 2025
15 of 16 checks passed
@kripken kripken deleted the oi.rgd branch September 4, 2025 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants