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

Build fails on 32-bit systems #1509

Closed
cinerea0 opened this issue Jun 10, 2022 · 8 comments
Closed

Build fails on 32-bit systems #1509

cinerea0 opened this issue Jun 10, 2022 · 8 comments

Comments

@cinerea0
Copy link

While attempting to build scryer in CI, the build for i686 failed. Here's the error I encountered:

error[E0080]: evaluation of constant value failed
  --> src/atom_table.rs:22:1
   |
22 | const_assert!(mem::size_of::<Atom>() == 8);
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempt to compute `0_usize - 1_usize`, which would overflow
   |
   = note: this error originates in the macro `const_assert` (in Nightly builds, run with -Z macro-backtrace for more info)
error[E0080]: evaluation of constant value failed
   --> src/arena.rs:459:1
    |
459 | const_assert!(mem::size_of::<AllocSlab>() == 16);
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ attempt to compute `0_usize - 1_usize`, which would overflow
    |
    = note: this error originates in the macro `const_assert` (in Nightly builds, run with -Z macro-backtrace for more info)
For more information about this error, try `rustc --explain E0080`.

This brought me to the following line of code: https://github.com/mthom/scryer-prolog/blob/master/src/atom_table.rs#L22. This prohibits building on 32-bit systems due to the smaller usize. Is this intentional because there are parts of scryer that don't work on 32-bit systems?

@triska
Copy link
Contributor

triska commented Jun 10, 2022

Scryer needs 64 bits for its representation of WAM cells, see for example: #908 (comment)

@ericonr
Copy link

ericonr commented Jun 10, 2022

If the representation needs to be 64 bits, is there a reason it can't use u64 instead of usize?

@triska
Copy link
Contributor

triska commented Jun 10, 2022

If that makes it possible to use Scryer Prolog on 32-bit systems, and at the same time has no significant performance downsides on 64-bit systems, then I think this would be an awesome solution!

@classabbyamp
Copy link

there would be no effect, probably, because usize is a u64 already on 64-bit systems

@triska
Copy link
Contributor

triska commented Jun 23, 2023

Scryer's memory management needs 64 bits to represent a WAM cell, so it needs u64 in all systems where we want it to run. On 32-bit systems, usize is u32, so using usize does not work there, even though it coincidentally works on 64-bit systems.

@classabbyamp
Copy link

Right, so switching usize to u64 would incur no penalty to existing 64bit and have the added benefit of supporting 32bit

rujialiu pushed a commit to rujialiu/scryer-prolog that referenced this issue Aug 22, 2023
mthom added a commit that referenced this issue Aug 25, 2023
@triska
Copy link
Contributor

triska commented Aug 25, 2023

@rujialiu has impressively resolved this via #1972, could you please try it, and if possible re-enable the 32-bit target (void-linux/void-packages#37478) and close this issue if this now works for you? Thank you a lot!

@cinerea0
Copy link
Author

cinerea0 commented Sep 1, 2023

I can confirm this has been fixed in the latest release due to the incredible efforts of @rujialiu; build logs in this PR: void-linux/void-packages#45861. Also, since this project no longer depends on gmp, we were able to enable crossbuilds. Closing now, thank you again!

@cinerea0 cinerea0 closed this as completed Sep 1, 2023
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

4 participants