Title: [203851] trunk/Source/_javascript_Core
Revision
203851
Author
msab...@apple.com
Date
2016-07-28 17:30:32 -0700 (Thu, 28 Jul 2016)

Log Message

ARM64: Fused left shift with a right shift can create NaNs from integers
https://bugs.webkit.org/show_bug.cgi?id=160329

Reviewed by Geoffrey Garen.

When we fuse a left shift and a right shift of integers where the shift amounts
are the same and the size of the quantity being shifted is 8 bits, we rightly
generate a sign extend byte instruction.  On ARM64, we were sign extending
to a 64 bit quantity, when we really wanted to sign extend to a 32 bit quantity.

Checking the ARM64 marco assembler and we were extending to 64 bits for all
four combinations of zero / sign and 8 / 16 bits.
        
* assembler/MacroAssemblerARM64.h:
(JSC::MacroAssemblerARM64::zeroExtend16To32):
(JSC::MacroAssemblerARM64::signExtend16To32):
(JSC::MacroAssemblerARM64::zeroExtend8To32):
(JSC::MacroAssemblerARM64::signExtend8To32):
* tests/stress/regress-160329.js: New test added.
(narrow):

Modified Paths

Added Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (203850 => 203851)


--- trunk/Source/_javascript_Core/ChangeLog	2016-07-28 23:52:42 UTC (rev 203850)
+++ trunk/Source/_javascript_Core/ChangeLog	2016-07-29 00:30:32 UTC (rev 203851)
@@ -1,3 +1,26 @@
+2016-07-28  Michael Saboff  <msab...@apple.com>
+
+        ARM64: Fused left shift with a right shift can create NaNs from integers
+        https://bugs.webkit.org/show_bug.cgi?id=160329
+
+        Reviewed by Geoffrey Garen.
+
+        When we fuse a left shift and a right shift of integers where the shift amounts
+        are the same and the size of the quantity being shifted is 8 bits, we rightly
+        generate a sign extend byte instruction.  On ARM64, we were sign extending
+        to a 64 bit quantity, when we really wanted to sign extend to a 32 bit quantity.
+
+        Checking the ARM64 marco assembler and we were extending to 64 bits for all
+        four combinations of zero / sign and 8 / 16 bits.
+        
+        * assembler/MacroAssemblerARM64.h:
+        (JSC::MacroAssemblerARM64::zeroExtend16To32):
+        (JSC::MacroAssemblerARM64::signExtend16To32):
+        (JSC::MacroAssemblerARM64::zeroExtend8To32):
+        (JSC::MacroAssemblerARM64::signExtend8To32):
+        * tests/stress/regress-160329.js: New test added.
+        (narrow):
+
 2016-07-28  Mark Lam  <mark....@apple.com>
 
         StringView should have an explicit m_is8Bit field.

Modified: trunk/Source/_javascript_Core/assembler/MacroAssemblerARM64.h (203850 => 203851)


--- trunk/Source/_javascript_Core/assembler/MacroAssemblerARM64.h	2016-07-28 23:52:42 UTC (rev 203850)
+++ trunk/Source/_javascript_Core/assembler/MacroAssemblerARM64.h	2016-07-29 00:30:32 UTC (rev 203851)
@@ -1125,12 +1125,12 @@
 
     void zeroExtend16To32(RegisterID src, RegisterID dest)
     {
-        m_assembler.uxth<64>(dest, src);
+        m_assembler.uxth<32>(dest, src);
     }
 
     void signExtend16To32(RegisterID src, RegisterID dest)
     {
-        m_assembler.sxth<64>(dest, src);
+        m_assembler.sxth<32>(dest, src);
     }
 
     void load8(ImplicitAddress address, RegisterID dest)
@@ -1190,12 +1190,12 @@
 
     void zeroExtend8To32(RegisterID src, RegisterID dest)
     {
-        m_assembler.uxtb<64>(dest, src);
+        m_assembler.uxtb<32>(dest, src);
     }
 
     void signExtend8To32(RegisterID src, RegisterID dest)
     {
-        m_assembler.sxtb<64>(dest, src);
+        m_assembler.sxtb<32>(dest, src);
     }
 
     void store64(RegisterID src, ImplicitAddress address)

Added: trunk/Source/_javascript_Core/tests/stress/regress-160329.js (0 => 203851)


--- trunk/Source/_javascript_Core/tests/stress/regress-160329.js	                        (rev 0)
+++ trunk/Source/_javascript_Core/tests/stress/regress-160329.js	2016-07-29 00:30:32 UTC (rev 203851)
@@ -0,0 +1,17 @@
+// Regression test for 160329. This test should not crash or throw an exception.
+
+function narrow(x) {
+    return x << 24 >> 24;
+}
+
+noInline(narrow);
+
+for (var i = 0; i < 1000000; i++) {
+    let expected = i << 24;
+    let got = narrow(i);
+    expected = expected >> 24;
+
+    if (expected != got)
+        throw "FAILURE, expected:" + expected + ", got:" + got;
+}
+
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to