Bug 3115 – >>> and >>>= generate wrong code

Illegal optimization to unsigned right shift (only array) // source code: test.d import std.stdio; int main(string[] args) { long a = -1; long[] b = [ -1 ]; writefln("a (%X) >>> 1 = %X", a , (a >>> 1)); writefln("b[0](%X) >>> 1 = %X", b[0], (b[0] >>> 1)); return 0; } ---- result (compile : dmd test.d) a (FFFFFFFFFFFFFFFF) >>> 1 = 7FFFFFFFFFFFFFFF // OK b[0](FFFFFFFFFFFFFFFF) >>> 1 = 7FFFFFFFFFFFFFFF // OK ---- result (compile : dmd -O test.d) a (FFFFFFFFFFFFFFFF) >>> 1 = 7FFFFFFFFFFFFFFF // OK b[0](FFFFFFFFFFFFFFFF) >>> 1 = FFFFFFFFFFFFFFFF // NG?

Here's a variation that doesn't even require the optimizer. In the code below, >>>= gets changed into >>=. ---- void main() { long[1] b = void; b[0] = -1L; b[0] >>>= 2; assert( (b[0]) == 0x3FFFFFFFFFFFFFFFL); }

This is happening because the backend doesn't constant-fold >>> correctly. Consequently, there's incorrect code in a few places because of this. Sometimes OPshr means >>, sometimes it means >>>. The patch below changes it so that OPshr always means >>>. OPashr means >> in the case where the value is signed. The code in evalu8.c is the most important. This patch may cause problems for DMC. Some of these changes might need to be inside #if MARS/#endif blocks. Some test cases: (test with both dmd bug.d and dmd -O bug.d). long bar() { return -1L; } void main() { long[1] b = void; b[0]= bar(); assert((b[0]>>>2) == 0x3FFFFFFFFFFFFFFFL); assert((b[0]>>2) == 0xFFFFFFFFFFFFFFFFL); b[0] = -1L; assert((b[0]>>>2) == 0x3FFFFFFFFFFFFFFFL); assert((b[0]>>2) == 0xFFFFFFFFFFFFFFFFL); b[0] = -1L; b[0] >>>= 2; assert( (b[0]) == 0x3FFFFFFFFFFFFFFFL); b[0] = -1L; b[0] >>= 2; assert( (b[0]) == 0xFFFFFFFFFFFFFFFFL); } ---- PATCH: Index: backend/cod2.c =================================================================== --- backend/cod2.c (revision 252) +++ backend/cod2.c (working copy) @@ -1847,8 +1847,8 @@ oper = e->Eoper; // Do this until the rest of the compiler does OPshr/OPashr correctly - if (oper == OPshr) - oper = (uns) ? OPshr : OPashr; +// if (oper == OPshr) +// oper = (uns) ? OPshr : OPashr; switch (oper) { case OPshl: Index: backend/cod4.c =================================================================== --- backend/cod4.c (revision 252) +++ backend/cod4.c (working copy) @@ -1367,10 +1367,9 @@ *pretregs |= ALLREGS; // Do this until the rest of the compiler does OPshr/OPashr correctly - if (oper == OPshrass) - oper = (uns) ? OPshrass : OPashrass; +// if (oper == OPshrass) +// oper = (uns) ? OPshrass : OPashrass; - // Select opcodes. op2 is used for msw for long shifts. switch (oper) Index: backend/evalu8.c =================================================================== --- backend/evalu8.c (revision 252) +++ backend/evalu8.c (working copy) @@ -1533,10 +1533,22 @@ } else { //printf("signed\n"); + e->EV.Vllong = ((targ_ullong)l1) >> i2; + } + break; + + case OPashr: + if ((targ_ullong) i2 > sizeof(targ_ullong) * 8) + i2 = sizeof(targ_ullong) * 8; + if (tyuns(tym)) + { //printf("unsigned\n"); + e->EV.Vullong = ((targ_ullong) l1) >> i2; + } + else + { //printf("signed\n"); e->EV.Vllong = l1 >> i2; } break; - case OPpair: switch (tysize[tym]) { Index: e2ir.c =================================================================== --- e2ir.c (revision 252) +++ e2ir.c (working copy) @@ -3007,7 +3007,7 @@ elem *ShrAssignExp::toElem(IRState *irs) { - return toElemBin(irs,OPshrass); + return toElemBin(irs, tyuns(type->ty) ? OPshrass : OPashrass); } @@ -3117,7 +3117,7 @@ elem *ShrExp::toElem(IRState *irs) { - return toElemBin(irs,OPshr); + return toElemBin(irs, tyuns(type->ty) ? OPshr : OPashr); }

SVN commit: http://www.dsource.org/projects/dmd/changeset/266

Fixed dmd 1.053 and 2.037