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

Shift right would overflow #4

Closed
sengerts opened this issue Nov 25, 2020 · 4 comments · Fixed by #5
Closed

Shift right would overflow #4

sengerts opened this issue Nov 25, 2020 · 4 comments · Fixed by #5

Comments

@sengerts
Copy link
Contributor

I am getting the following errors in the bitboard module while compiling the source code:
image

The code affected by these errors is the following:

// Compute the attack's index using the 'magic bitboards' approach
fn index_bishop(s: Square, occupied: Bitboard) -> usize {
    unsafe {
        u64::wrapping_mul((occupied & BISHOP_MAGICS.masks[s.0 as usize]).0,
            BISHOP_MAGICS.magics[s.0 as usize]) as usize >> (64-9)
    }
}

fn index_rook(s: Square, occupied: Bitboard) -> usize {
    unsafe {
        u64::wrapping_mul((occupied & ROOK_MAGICS.masks[s.0 as usize]).0,
            ROOK_MAGICS.magics[s.0 as usize]) as usize >> (64-12)
    }
}

Do you know why this error occurs or what I can do to fix the overflow?

@sengerts
Copy link
Contributor Author

Can I jsut use a wrapping_shr on the usize value or will this differ from the intended semantics?

@niklasf
Copy link

niklasf commented Nov 25, 2020

Looks like usize has less than 64 bits on your platform. The intention is to do the shift first in u64, then cast to usize:

(u64::wrapping_mul(
    (occupied & ROOK_MAGICS.masks[s.0 as usize]).0,
    ROOK_MAGICS.magics[s.0 as usize]
) >> (64-12)) as usize

@syzygy1
Copy link
Owner

syzygy1 commented Nov 26, 2020

Yes, this looks like a bug, and @niklasf 's fix seems correct.

@sengerts
Copy link
Contributor Author

Thank you both! I have forked the repo and created a pull request.

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 a pull request may close this issue.

3 participants