www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 3115] New: Illegal optimization to unsigned right shift (only array)

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3115

           Summary: Illegal optimization to unsigned right shift (only
                    array)
           Product: D
           Version: 2.030
          Platform: x86
        OS/Version: Windows
            Status: NEW
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: sys0tem msn.com


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?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Jun 30 2009
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3115


Don <clugdbug yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |clugdbug yahoo.com.au


--- Comment #1 from Don <clugdbug yahoo.com.au> 2009-11-18 05:18:30 PST ---
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); } -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Nov 18 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3115


Don <clugdbug yahoo.com.au> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch
            Summary|Illegal optimization to     |>>> and >>>= generate wrong
                   |unsigned right shift (only  |code
                   |array)                      |


--- Comment #2 from Don <clugdbug yahoo.com.au> 2009-11-19 01:36:12 PST ---
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);
 }

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 19 2009
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3115


Leandro Lucarella <llucax gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |llucax gmail.com


--- Comment #3 from Leandro Lucarella <llucax gmail.com> 2009-11-23 06:48:52
PST ---
SVN commit: http://www.dsource.org/projects/dmd/changeset/266

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Nov 23 2009
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=3115


Walter Bright <bugzilla digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
                 CC|                            |bugzilla digitalmars.com
         Resolution|                            |FIXED


--- Comment #4 from Walter Bright <bugzilla digitalmars.com> 2009-12-06
00:45:54 PST ---
Fixed dmd 1.053 and 2.037

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Dec 06 2009