>> - checksum = (u16)IXGBE_EEPROM_SUM - checksum; >> + checksum = IXGBE_EEPROM_SUM - checksum; >> > Can't lead to different results, especially when: > checksum > IXGBE_EEPROM_SUM → the result becomes negative in int, and > narrowing to u16 causes unexpected wraparound? > > With this patch you are changing the semantics of the code - from explicit > 16bit arithmetic to full int implicit promotion which can be error-prone or > compiler-dependent /* for different targets */.
As far as I understand the C language does this by design - in the terms of C specification: > If an int can represent all values of the original type (...), the value is > converted to an int; otherwise, it is converted to an unsigned int. These are > called the integer promotions. (see note) > > (note) The integer promotions are applied only: as part of the usual > arithmetic conversions, (...) And subtraction semantics are: > If both operands have arithmetic type, the usual arithmetic conversions are > performed on them. So there is no *16 bit arithmetic* - it is always done on integers (usually 32 bits). Or have I missed something? Additionally I've checked AMD64, ARM and MIPS assembly output from GCC and clang on https://godbolt.org/z/GPsMxrWfe - both of the following snippets compile to exactly the same assembly: #define NVM_SUM 0xBABA int test(int num) { volatile unsigned short test = 0xFFFF; unsigned short checksum = NVM_SUM - test; return checksum; } vs.: #define NVM_SUM 0xBABA int test(int num) { volatile unsigned short test = 0xFFFF; unsigned short checksum = ((unsigned short)NVM_SUM) - test; return checksum; } -- Best regards, Jacek Kowalski
