-
Notifications
You must be signed in to change notification settings - Fork 20
Unnecessary wrapping_add? #4
Comments
Not in debug mode; it would panic in that mode when an overflow occurs. The wrapping_add makes sure there's no panic branch even in debug mode.
Oh, wow. Is that UB?
I answered over there. |
Good point, I didn't think about debug mode. From what I can find on stackoverflow, yes, signed overflow is still undefined, but it sounds like practically all platforms use two complement that wraps around: |
I noticed that you're manually calling wrapping_* for arithmetic operations in the PTX kernels. Shouldn't overflow wrap around already?
Note that I also noticed that it appears the intrinsics use signed for these variables (for which overflow would be undefined in C, although defined as wraparound in RFC 560 for Rust) but are defined as using unsigned (defined as wraparound in both C and Rust) in the CUDA programming manual. I filed a ticket in that library about this.
https://github.com/nox/rust-rfcs/blob/master/text/0560-integer-overflow.md
japaric-archived/nvptx-builtins#1
Feel free to close the ticket if it isn't relevant - I don't typically do GPGPU programming and this was the easiest way for me to provide feedback.
The text was updated successfully, but these errors were encountered: