Merge r184414 - [ARM64] Do not fail branchConvertDoubleToInt32 when t…

…he result is zero and not negative zero https://bugs.webkit.org/show_bug.cgi?id=144976 Patch by Benjamin Poulain <bpoulain@apple.com> on 2015-05-15 Reviewed by Michael Saboff. Failing the conversion on zero is pretty dangerous as we discovered on x86. This patch does not really impact performance significantly because r184220 removed the zero checks from Kraken. This patch is just to be on the safe side for cases not covered by existing benchmarks. * assembler/MacroAssemblerARM64.h: (JSC::MacroAssemblerARM64::branchConvertDoubleToInt32):
WebKit · Jul 6, 2015 · 6007774 · 6007774
1 parent e83219d
commit 6007774
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 3 deletions.
diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
@@ -1,3 +1,19 @@
+2015-05-15  Benjamin Poulain  <bpoulain@apple.com>
+
+        [ARM64] Do not fail branchConvertDoubleToInt32 when the result is zero and not negative zero
+        https://bugs.webkit.org/show_bug.cgi?id=144976
+
+        Reviewed by Michael Saboff.
+
+        Failing the conversion on zero is pretty dangerous as we discovered on x86.
+
+        This patch does not really impact performance significantly because
+        r184220 removed the zero checks from Kraken. This patch is just to be
+        on the safe side for cases not covered by existing benchmarks.
+
+        * assembler/MacroAssemblerARM64.h:
+        (JSC::MacroAssemblerARM64::branchConvertDoubleToInt32):
+
 2015-05-14  Andreas Kling  <akling@apple.com>
 
         String.prototype.split() should create efficient substrings.

diff --git a/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h b/Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
@@ -1193,9 +1193,14 @@ class MacroAssemblerARM64 : public AbstractMacroAssembler<ARM64Assembler, MacroA
         m_assembler.scvtf<64, 32>(fpTempRegister, dest);
         failureCases.append(branchDouble(DoubleNotEqualOrUnordered, src, fpTempRegister));
 
-        // If the result is zero, it might have been -0.0, and the double comparison won't catch this!
-        if (negZeroCheck)
-            failureCases.append(branchTest32(Zero, dest));
+        // Test for negative zero.
+        if (negZeroCheck) {
+            Jump valueIsNonZero = branchTest32(NonZero, dest);
+            RegisterID scratch = getCachedMemoryTempRegisterIDAndInvalidate();
+            m_assembler.fmov<64>(scratch, src);
+            failureCases.append(makeTestBitAndBranch(scratch, 63, IsNonZero));
+            valueIsNonZero.link(this);
+        }
     }
 
     Jump branchDouble(DoubleCondition cond, FPRegisterID left, FPRegisterID right)