Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Opcode for delegate #145

Closed
takikawa opened this issue Jan 12, 2021 · 2 comments
Closed

Opcode for delegate #145

takikawa opened this issue Jan 12, 2021 · 2 comments

Comments

@takikawa
Copy link
Collaborator

takikawa commented Jan 12, 2021 •
edited
Loading

In #137 it's mentioned that the opcode for delegate needs to be decided. Has there been any discussion of that yet?

As mentioned in #137, the 0x0? opcodes are used up. I believe the V8 code is currently using 0x16 as a placeholder, but I think that and 0x17 are used by the function references proposal.

It looks like 0x18 or 0x19 may be available though based on looking through the current proposals.
@aheejin
Copy link
Member

aheejin commented Jan 12, 2021

Following V8 implementation, I was tentatively using 0x16 in my toolchain implementation, but given that it is already takne, we should probably change it to 0x18 or 0x19 as you suggested. How about 0x18? @thibaudmichaud

@thibaudmichaud
Copy link
Collaborator

Thanks for pointing out the conflict with function references, @takikawa. 0x18 sounds good.

@Ms2ger Ms2ger mentioned this issue Aug 4, 2022
Open
pull bot pushed a commit to p-g-krish/v8 that referenced this issue Jan 15, 2021
@thibaudmichaud
[wasm][eh] Fix delegate opcode
1f1710d 
Replace 0x16 with 0x18 for the delegate opcode, to avoid a conflict with
the function reference proposal.
See WebAssembly/exception-handling#145

[email protected]

Bug: v8:8091
Change-Id: Ib012f8680dfece200973e18fdf6c82877f10d5de
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2632604
Reviewed-by: Clemens Backes <[email protected]>
Commit-Queue: Thibaud Michaud <[email protected]>
Cr-Commit-Position: refs/heads/master@{#72118}
This was referenced Feb 9, 2021
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@takikawa @thibaudmichaud @aheejin