cc65: Inconsistent result on divide with a negative numerator and a constant or variable denominator
When dividing a negative integer numerator by a constant number, or by a variable containing that same number, the compiler generates different results.
This sample code illustrates, and will erroneously print “Not Equal”:
#include <stdio.h>
void main(void)
{
int a = -1, b = 8;
if ((a/8) == (a/b))
printf("Equal\n");
else
printf("Not Equal\n");
}
Looking at the generated code, it’s easy to see where the error lies. The compiler substituted a shift operation for the divide-by-a-constant. The shift operation doesn’t yield the same answer as the divide if the numerator is negative. For the case above, -1 >> 3 = -1 and -1 / 8 = 0.
;
; if ((a/8) == (a/b))
;
ldy #$03
jsr ldaxysp
jsr asrax3 ; use >> 3 to divide by 8
jsr pushax
ldy #$05
jsr ldaxysp
jsr pushax
ldy #$05
jsr ldaxysp
jsr tosdivax ; use divide code to divide by 8
The optimizer switched to using asrax3 for the constant division; and, used tosdivax for the division by the variable, resulting in -1/8 not giving a consistent result, depending on whether the 8 is used as a literal or in a variable.
About this issue
- Original URL
- State: closed
- Created 9 years ago
- Comments: 15 (15 by maintainers)
Commits related to this issue
- Almost fixed Issue #169. The only denominator not working right now is -2147483648. — committed to acqn/cc65 by acqn 4 years ago
- More optimized codegen for the correct cases of the Issue #169 fix. — committed to acqn/cc65 by acqn 4 years ago
- Added support for changing divisions by negative power-of-2 denominators into bit shifts, and fixed #169 including the case of -2147483648 which is negative but appears positive. — committed to acqn/cc65 by acqn 4 years ago
- Almost fixed Issue #169. The only denominator not working right now is -2147483648. — committed to cc65/cc65 by acqn 4 years ago
- More optimized codegen for the correct cases of the Issue #169 fix. — committed to cc65/cc65 by acqn 4 years ago
- tests for issue #169 — committed to cc65/cc65 by mrdudz 4 years ago
Thank you for the updates. I modified them a bit to test negative/negative as well and forgoe testing with +128, +32768 and +2147483648 when these numbers exceed the corresponding ranges of the tested signed data types.
The new 32-bit signed long testing got me with -2147483648L and it seemed similar to the hexidemical unsigned promotion. As the integer promotion rule is confusing to me too, even with the decimal constants, I investigated it a bit more, and it surprised me that, there are no “negative integer literals”, but just “negated integer literals”.
For example, you can’t have a negative integer literal “-128”, but just a “-(128)”, which is a negation expression of the positive integer constant 128.
As this negation is carried out after the compiler has determined the data type, it could give some surprise when the constant is so large that it is promoted to unsigned, whose negation still gives an unsigned constant. In C89/90, 2147483648L is unsigned long (but signed long long in later standards), so -2147483648L is still unsigned long (and signed long long in later standards) and is actually (unsigned long)-2147483648UL . To get a signed -2147483648, you have to cast the expression to signed long (if you can’t use the long long type/LL suffix). Yeah another pitfall in C.
I’ll check if there’re anything that I still overlooked with the test.
Thank you! It looks good (and already helps), though I think typedef or the stdint.h stuff would be better for the reference compiler.
EDIT: I once suspected if the type deduction in cc65 was wrong, but it turns out that I’ve been living in a world where int is 32-bit for too long: In cc65, int is 16-bit, and both 0x8000 and -0x8000 are actually unsigned (due to some special rule on hex constants). So the correct results of test14a and test14b from the div-int-int test in their current forms are not 0, but 1, because -0x7fff gets “co-promoted” to unsigned int too, which becomes 0x8001u that is larger than 0x8000u. To get the “expected test result” 0 in cc65, 0x8000 must be cast to signed int (if it’s meant to be -0x8000) or signed long (if it’s really meant to be signed +0x8000) if you want to keep the hexadecimal form, or just write it as decimal 32768. That’s my two cents.
EDIT: I just forgot about the obvious: only the hexadecimal numbers are interpreted as unsigned in such a way, and the decimal numbers are normal. Updated the previous edit with regard to that.