nvptx icon indicating copy to clipboard operation
nvptx copied to clipboard

Unnecessary wrapping_add?

Open beamspease opened this issue 7 years ago • 2 comments

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 https://github.com/japaric/nvptx-builtins/issues/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.

beamspease avatar May 19 '17 23:05 beamspease

I noticed that you're manually calling wrapping_* for arithmetic operations in the PTX kernels. Shouldn't overflow wrap around already?

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.

for which overflow would be undefined in C

Oh, wow. Is that UB?

I filed a ticket in that library about this.

I answered over there.

japaric avatar May 20 '17 00:05 japaric

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: http://stackoverflow.com/questions/18195715/why-is-unsigned-integer-overflow-defined-behavior-but-signed-integer-overflow-is http://stackoverflow.com/questions/18195715/why-is-unsigned-integer-overflow-defined-behavior-but-signed-integer-overflow-is http://stackoverflow.com/questions/19842215/wrap-around-explanation-for-signed-and-unsigned-variables-in-c

beamspease avatar May 20 '17 00:05 beamspease