Skip to content

Remove unnecessary MEMORY64 block#18209

Merged
sbc100 merged 1 commit intomainfrom
remove_unneeded_cast
Nov 18, 2022
Merged

Remove unnecessary MEMORY64 block#18209
sbc100 merged 1 commit intomainfrom
remove_unneeded_cast

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented Nov 16, 2022

The only reason this might be useful is of the EM_ASM block happens to return a bigint, which, in general, should not be that case.

@sbc100 sbc100 requested a review from kripken November 16, 2022 00:29
@sbc100 sbc100 enabled auto-merge (squash) November 16, 2022 00:29
@kripken
Copy link
Member

kripken commented Nov 16, 2022

Why can't an EM_ASM return a bigint?

@sbc100
Copy link
Collaborator Author

sbc100 commented Nov 16, 2022

Why can't an EM_ASM return a bigint?

I mean it could... but it could return a bigint without MEMORY64 too. I guess that question really is, why was this block added in the first place? I think that answer is that it used to be that we treated pointers as bigint under wasm64, so it make it more likely. Nowadays we turn pointers into int53 numbers at the boundary so it no more likely that an EM_ASM block would return a bigint under wasm64 or wasm32.

@kripken
Copy link
Member

kripken commented Nov 16, 2022

I see. Makes sense. Lgtm if we have a test of returning a pointer from an EM_ASM that works in MEMORY64 mode.

@sbc100
Copy link
Collaborator Author

sbc100 commented Nov 16, 2022

I see. Makes sense. Lgtm if we have a test of returning a pointer from an EM_ASM that works in MEMORY64 mode.

We actually have a separate EM_ASM_PTR and emscripten_asm_const_ptr to the times when you want to return a pointer.

@kripken
Copy link
Member

kripken commented Nov 16, 2022

Nice, I didn't know we had EM_ASM_PTR.

@sbc100 sbc100 force-pushed the remove_unneeded_cast branch from ad3c632 to e684698 Compare November 16, 2022 19:36
@sbc100
Copy link
Collaborator Author

sbc100 commented Nov 16, 2022

I think I'm going to land #18218 first which refactors stuff here.

The only reason this might be useful is of the EM_ASM block happens
to return a bigint, which, in general, should not be that case.
@sbc100 sbc100 force-pushed the remove_unneeded_cast branch from e684698 to 49ce3e5 Compare November 18, 2022 18:53
@sbc100 sbc100 merged commit 2e02d0e into main Nov 18, 2022
@sbc100 sbc100 deleted the remove_unneeded_cast branch November 18, 2022 20:11
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