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.

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

NaN boxing on 64 bit systems #2476

Closed
AE9RB opened this issue Jul 16, 2014 · 5 comments
Closed

NaN boxing on 64 bit systems #2476

AE9RB opened this issue Jul 16, 2014 · 5 comments

Comments

@AE9RB
Copy link
Contributor

AE9RB commented Jul 16, 2014

I noticed that NaN boxing was originally supporting 64 bit pointers and later removed. I couldn't find why this was the case but thought maybe it had to do with this:

AMD architecture uses only the lower 48 bits of an address, and bits 48 through 63 must be a copy of bit 47 or the processor raises an exception.

So maybe it's possible to add it back with this knowledge. I don't have an AMD core to test.

@cremno
Copy link
Contributor

cremno commented Jul 17, 2014

It was actually only 32-bit at first. But when was it removed? Looking at the header, NaN boxing seems to be supported by (some) 64-bit architectures, or am I missing something?

@AE9RB
Copy link
Contributor Author

AE9RB commented Jul 17, 2014

You can't compile NaN boxing on 64-bit right now because of:

mruby/src/state.c

Lines 32 to 34 in d17506c

#ifdef MRB_NAN_BOXING
mrb_static_assert(sizeof(void*) == 4, "when using NaN boxing sizeof pointer must be 4 byte");
#endif

@cremno
Copy link
Contributor

cremno commented Jul 17, 2014

Ah! Yes, that assertion should be removed. It was added before 64-bit support was merged.

ae288fe (Jun 2013)
#1441 (Aug 2013)

Do you want to create a PR or should I? 64-bit NaN boxing compiles and works fine for me.

@matz
Copy link
Member

matz commented Jul 17, 2014

64bit NaN boxing never had been implemented for mruby, but for LuaJIT which I steal NaN boxing idea. Since I don't have 64bit machine that only use lower 48 bits for sure, it's too hard for me to implement it. If someone comes with the patch, I am happy to merge.

@matz
Copy link
Member

matz commented Jul 19, 2014

Oops, I forgot about #1441. I will fix.

@matz matz closed this as completed in 4cc9e5d Jul 21, 2014
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

No branches or pull requests

3 participants