Bug: Word64.ror with offset 0w32 fails on x86

View: New views
2 Messages — Rating Filter:   Alert me  

Bug: Word64.ror with offset 0w32 fails on x86

by Wesley W. Terpstra :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

val x = valOf (Word64.fromString "1234567890abcdef")
val y = MLton.Word64.ror (x, 0w32)
val z = Word64.orb (Word64.<< (x, 0w32), Word64.>> (x, 0w32))

val () = print (Word64.toString y ^ "\n")
val () = print (Word64.toString z ^ "\n")

Output:
1234567890ABCDEF
90ABCDEF12345678

Expected output:
90ABCDEF12345678
90ABCDEF12345678

I've checked that this bug is NOT from gcc by compiling the following:
#include <stdint.h>
#include <stdio.h>

uint64_t Word64_ror(uint64_t w1, uint32_t w2);

int main () {
  uint64_t x = Word64_ror(0x1234567890ABCDEFUL, 32);
  printf ("%llX\n", x);
  return 0;
}
gcc -Wall -O2 bug.c /usr/lib/mlton/self/libmlton.a -o bug

Output:
90ABCDEF12345678

The number 0w32 strikes me as a case that someone probably optimized incorrectly.


_______________________________________________
MLton mailing list
MLton@...
http://mlton.org/mailman/listinfo/mlton

Re: Bug: Word64.ror with offset 0w32 fails on x86

by Wesley W. Terpstra :: Rate this Message:

Reply to Author | View Threaded | Show Only this Message

On Mon, Sep 14, 2009 at 9:07 PM, Wesley W. Terpstra <wesley@...> wrote:
val x = valOf (Word64.fromString "1234567890abcdef")
val y = MLton.Word64.ror (x, 0w32)
val z = Word64.orb (Word64.<< (x, 0w32), Word64.>> (x, 0w32))

val () = print (Word64.toString y ^ "\n")
val () = print (Word64.toString z ^ "\n")

Output:
1234567890ABCDEF
90ABCDEF12345678
The number 0w32 strikes me as a case that someone probably optimized incorrectly.

I believe the bug comes from atoms/prim.fun. There is an optimization there which eliminates rol/ror by a multiple of the word size. However, I believe it is using the wordsize of the shift argument instead of the value argument. The attached patch is my solution. I'll commit it after testing.


[rolr-bug.patch]

Index: prim.fun
===================================================================
--- prim.fun (revision 7219)
+++ prim.fun (working copy)
@@ -1861,23 +1861,19 @@
                                 then Var x
                              else Apply (neg s, [x])
                      else Unknown
-                  fun ro () =
+                  fun ro s =
                      if inOrder
                         then
-                           let
-                              val s = WordX.size w
-                           in
-                              if WordX.isZero
-                                 (WordX.rem
-                                  (w,
-                                   WordX.fromIntInf
-                                   (IntInf.fromInt
-                                    (Bits.toInt (WordSize.bits s)),
-                                    s),
-                                   {signed = false}))
-                                 then Var x
-                              else Unknown
-                           end
+                           if WordX.isZero
+                              (WordX.rem
+                               (w,
+                                WordX.fromIntInf
+                                (IntInf.fromInt
+                                 (Bits.toInt (WordSize.bits s)),
+                                 WordX.size w),
+                                {signed = false}))
+                              then Var x
+                           else Unknown
                      else
                         if WordX.isZero w orelse WordX.isAllOnes w
                            then word w
@@ -1944,8 +1940,8 @@
                                     orelse signed andalso WordX.isNegOne w)
                            then zero s
                         else Unknown
-                   | Word_rol _ => ro ()
-                   | Word_ror _ => ro ()
+                   | Word_rol s => ro s
+                   | Word_ror s => ro s
                    | Word_rshift (s, {signed}) =>
                         if signed
                            then


_______________________________________________
MLton mailing list
MLton@...
http://mlton.org/mailman/listinfo/mlton