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

Fix comptime_int bitwise operators (#1387) #1529

Closed
wants to merge 7 commits into
base: master
from

Conversation

Projects
None yet
2 participants
@kristate
Contributor

kristate commented Sep 15, 2018

Fixes #1387

#1387

  • i64(3 & -1) == 3;
  • i64(3 & 1) == 1;
  • i64(-3 & -1) == -3;
  • i64(3 | -1) == -1;
  • i64(3 ^ -1) == -4;

@kristate kristate referenced this pull request Sep 15, 2018

Closed

Binary NOT Support for comptime_int (#1382) #1389

3 of 3 tasks complete

@kristate kristate force-pushed the kristate:comptime_int-bitwise-issue1387 branch from 774d579 to cc53f46 Sep 15, 2018

@andrewrk

can you squash your commits?

@@ -437,6 +435,21 @@ bool mul_u64_overflow(uint64_t op1, uint64_t op2, uint64_t *result) {
}
#endif
static inline void _twos_complement_inplace(BigInt *bn) {

This comment has been minimized.

@andrewrk

andrewrk Sep 15, 2018

Member

No explicit inline unless:

  • you need it so that you can put the function in a header file
  • you have a benchmark to demonstrate that it's faster and not too much code size increase
  • the function semantics depends on always being inlined. in this case you would need a compiler-specific attribute anyway.

Also, no need for leading underscore on static functions.

This comment has been minimized.

@kristate

kristate Sep 15, 2018

Contributor

Okay, I will change the signature.

bigint_init_unsigned(dest, 0);
return;
if (!bit_count) {
bit_count = (op->digit_count * 64) - bigint_clz(op, op->digit_count * 64) + (is_signed ? 1 : 0);

This comment has been minimized.

@andrewrk

andrewrk Sep 15, 2018

Member

why this change?

this function is supposed to evaluate binary not based on the bit count passed in as a parameter. So if the passed in value is 0 then binary not of 0 is 0.

This comment has been minimized.

@kristate

kristate Sep 15, 2018

Contributor

@andrewrk I will document this function.

So if the passed in value is 0 then binary not of 0 is 0.

I think that is an incorrect assumption -- bit_count only specifies how many bits should be included in the not operation; furthermore, BigInt is architected only to store a value. zig seems to have a u0 type, but I don't see how this is valuable or used.

This implementation correctly returns not of op into dest with optional bound of bit_count.

kristate added some commits Aug 26, 2018

src/bigint.cpp: If `bit_count` is zero, this function will operate on…
… the exact number of bits stored inside of the `BigInt`

@kristate kristate force-pushed the kristate:comptime_int-bitwise-issue1387 branch from cc53f46 to c09326d Sep 16, 2018

@andrewrk andrewrk added this to the 0.4.0 milestone Sep 21, 2018

@andrewrk andrewrk closed this in 32c91ad Sep 24, 2018

@andrewrk andrewrk modified the milestones: 0.4.0, 0.3.0 Sep 28, 2018

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