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

Initial implementation of the 128-bit integers #35954

Closed
wants to merge 15 commits into from

Conversation

nagisa
Copy link
Member

@nagisa nagisa commented Aug 24, 2016

This commit introduces 128-bit integers. Stage 2 builds and produces a working compiler which understands and correctly handles 128-bit integers.

The general strategy used is to have rustc_i128 module which provides aliases for iu128, equal to iu64 in stage0 and iu128 later. Since nowhere in rustc we rely on large numbers being supported, this strategy is good enough to get past the first bootstrap stages to end up with a fully working 128-bit capable compiler.

This PR contains no compiler-rt implementations yet (will get around implementing anything necessary before landing) and I feel that I didn’t quite catch all the locations where iu64 need to be converted to iu128 in the compiler yet. No effort were made to update the Makefiles (will get around it tomorrow, I hope) or old trans (since it is getting removed soon anyway) either.

During implementation the only really problematic issue which has been discovered is that there seems to be no way to generate debug info for 128-bit discriminants (DIBuilder::createEnumerator takes 64bit value), so I didn’t put much effort into supporting the i128 discriminants at all.

All being said, the stage2 test suite seems to pass on a x86_64 linux machine. I don’t have any means to test any other targets (either virtualised or not) by myself at least till early autumn.

cc #35118

[plugin-breaking-change] due to changes to the AST.

@rust-highfive
Copy link
Collaborator

r? @arielb1

(rust_highfive has picked a reviewer for you, use r? to override)

@nagisa nagisa force-pushed the i-give-you-one-twenty-eight branch from aa98158 to 16c01cb Compare August 24, 2016 00:17

pub trait Encoder {
type Error;

// Primitive types:
fn emit_nil(&mut self) -> Result<(), Self::Error>;
fn emit_uint(&mut self, v: usize) -> Result<(), Self::Error>;
fn emit_u128(&mut self, v: u128) -> Result<(), Self::Error>;
Copy link
Member

Choose a reason for hiding this comment

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

This is scary, but I think since rustc-serialize on crates.io is separate and deriving wasn't changed, it should work?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I guess. I’m more worried about a similar addition to the Hash trait.

@bors
Copy link
Contributor

bors commented Aug 25, 2016

☔ The latest upstream changes (presumably #35764) made this pull request unmergeable. Please resolve the merge conflicts.

@nagisa nagisa force-pushed the i-give-you-one-twenty-eight branch from 5a8e99d to 603d409 Compare August 25, 2016 22:36
@bors
Copy link
Contributor

bors commented Aug 26, 2016

☔ The latest upstream changes (presumably #35906) made this pull request unmergeable. Please resolve the merge conflicts.

@nagisa nagisa force-pushed the i-give-you-one-twenty-eight branch from 603d409 to dbcd45a Compare August 26, 2016 12:18
@bors
Copy link
Contributor

bors commented Aug 30, 2016

☔ The latest upstream changes (presumably #36066) made this pull request unmergeable. Please resolve the merge conflicts.

@est31
Copy link
Member

est31 commented Sep 26, 2016

Any news on this? It would be really great to have 128 bit integer support.

@nagisa
Copy link
Member Author

nagisa commented Sep 26, 2016

Was waiting on #35021 to land; didn’t notice it had already. May rebase and finish this tomorrow.

This commit introduces 128-bit integers. Stage 2 builds and produces a working compiler which
understands and supports 128-bit integers throughout.

The general strategy used is to have rustc_i128 module which provides aliases for iu128, equal to
iu64 in stage9 and iu128 later. Since nowhere in rustc we rely on large numbers being supported,
this strategy is good enough to get past the first bootstrap stages to end up with a fully working
128-bit capable compiler.

In order for this strategy to work, number of locations had to be changed to use associated
max_value/min_value instead of MAX/MIN constants as well as the min_value (or was it max_value?)
had to be changed to use xor instead of shift so both 64-bit and 128-bit based consteval works
(former not necessarily producing the right results in stage1).
Dangling a carrot in front of a donkey.
Stage 1 can’t really handle negative 128-bit literals, but an equivalent bit-not is fine
Also fix the leb128 tests
Fixes rebase fallout, makes code correct in presence of 128-bit constants.
Causes ICEs otherwise while trying to dump AST
@durka
Copy link
Contributor

durka commented Sep 29, 2016

Does this affect intrinsics::discriminant_value() and mem::discriminant() if the maximum discriminant size is now iu128? It kinda sucks to make such functions slow-by-default if most computers don't have 128-bit-wide registers and have to emulate it, but maybe that's not an issue.

@nagisa
Copy link
Member Author

nagisa commented Oct 2, 2016

Status update: I have some intrinsics ported, but I can’t get the multiplication to work for some reason, I’ll need more time to investigate that.

Does this affect intrinsics::discriminant_value() and mem::discriminant() if the maximum discriminant size is now iu128

Eventually we’ll want to make this happen. In optimised code LLVM should be pretty smart about not using the whole 128 bits when it doesn’t need to. Moreover storage of i128 is not that much slower than storage of i64, so it should be of little concern.

@nagisa nagisa force-pushed the i-give-you-one-twenty-eight branch from eeca54d to b2df420 Compare October 2, 2016 20:02
@alexcrichton
Copy link
Member

ping @nagisa, any update on this?

emberian added a commit to emberian/rust-crypto that referenced this pull request Nov 6, 2016
With a better algorithm (based on Horner's Method, as recommended
by Ted Krovetz), a bigint isn't necessary, just 64-64 multiplication.
This addresses the XXX_QUESTION around NH and Hash.

Further efficiency is gained by avoiding a physical modulo operation,
instead using the identity (a % (2^61 - 1)) = (a >> 61) + (a & (2^61 -1)),
and borrow's from Ted's reference implementation.

This pulls in the extprim crate as an interim measure until
rust-lang/rust#35954 is released, which
provides native 128-bit words.
@est31 est31 mentioned this pull request Nov 20, 2016
2 tasks
@nagisa
Copy link
Member Author

nagisa commented Nov 21, 2016

@est31 is working on finishing this in the issue referenced above.

@nagisa nagisa closed this Nov 21, 2016
bors added a commit that referenced this pull request Dec 8, 2016
i128 and u128 support

Adds support for 128-bit integers. Rebased version of #35954 , with additional improvements.

Thanks @nagisa for mentoring this PR!

Some of the problems that were resolved:

* [x] Confirm that intrinsics work on 32 bit platforms and fix them if needed

* Wait for #37906 to get fixed, so that work on intrinsics can be completed *(worked around by merging the PR on my local setup)*

* [x] Investigate and fix linkcheck failure

[plugin-breaking-change]

cc #35118
@est31 est31 mentioned this pull request Dec 20, 2016
bors added a commit that referenced this pull request Dec 31, 2016
i128 and u128 support

Brings i128 and u128 support to nightly rust, behind a feature flag. The goal of this PR is to do the bulk of the work for 128 bit integer support. Smaller but just as tricky features needed for stabilisation like 128 bit enum discriminants are left for future PRs.

Rebased version of  #37900, which in turn was a rebase + improvement of #35954 . Sadly I couldn't reopen #37900 due to github. There goes my premium position in the homu queue...

[plugin-breaking-change]

cc #35118 (tracking issue)
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.