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

Implement increment and decrement operations #7

Open
Robbepop opened this issue Dec 21, 2017 · 4 comments
Open

Implement increment and decrement operations #7

Robbepop opened this issue Dec 21, 2017 · 4 comments

Comments

@Robbepop
Copy link
Owner

To increase or decrease the value of an ApInt instance by one today a user has to instantiate another ApInt with the same bit width of value representing one (1) and add or subtract it from the given ApInt. The creation of the temporary ApInt might be a slow operations and this it is better to have new APIs ApInt::increment and ApInt::decrement to efficiently increase or decrease an ApInt by one (1).

@Robbepop
Copy link
Owner Author

Maybe this could be enriched to also cover checked_add_assign, checked_sub_assign, checked_mul_assign, checked_div_assign as well as checked_rem_assign and their associated into_checked_* methods for primitive types such as i8, u16, ... i64, u128 etc for performance reason.

@Robbepop
Copy link
Owner Author

Robbepop commented Jan 2, 2018

Maybe it is better to implement a more generic way to increment or decrement an ApInt with the following API:

  • fn increment_by(&mut self, val: N) where N: PrimitiveInt
  • fn decrement_by(&mut self, val: N) where N: PrimitiveInt

These methods are not named to their very similar counterparts ApInt::add_assign and ApInt::sub_assign since they do not need to handle errors due to differences in bit widths.

@AaronKutch
Copy link
Contributor

I will do a pull request for wrapping_dec. I could definitely also do wrapping_inc_by and its dec counterpart. Something I realized though is that there could be a family of wrapping_..._by functions that use only a particular optimized branch of their larger counterparts. For example, there are several use cases where large ApInts only need to be multiplied by a Digit (and there is a branch in wrapping_mul_assign' that does that), but unless there is a specialized wrapping_mul_byfunction there is inefficiency incurred in match statements and such. But the problem is, what would would the argument be, aDigit? Digit` is only public to the crate now, but it looks like something that could be made public in the future.

@Robbepop
Copy link
Owner Author

For the moment I am against implementing {increment|decrement}_by since there are interface and design problems with it for very small apint instances below 64 bit width. So I'd just go with wrapping_inc and wrapping_dec for now. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants