Attachment 220583 Details for Bug 126612 – Patch

[patch] Patch

126612.patch (text/plain), 3.42 KB, created by Michael Saboff on 2014-01-07 19:37:28 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Michael Saboff

Created: 2014-01-07 19:37:28 PST

Size: 3.42 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 161477)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,17 @@
>+2014-01-07  Michael Saboff  <msaboff@apple.com>
>+
>+        CStack Branch: ARM64 DFG produces the wrong answer for X % 0
>+        https://bugs.webkit.org/show_bug.cgi?id=126612
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Added check for zero divisor.  If shouldCheckOverflow() is true, the zero divisor
>+        check is an overflow speculation check.  If shouldCheckOverflow() is false, a
>+        zero divisor causes us to set the result to 0.
>+
>+        * dfg/DFGSpeculativeJIT.cpp:
>+        (JSC::DFG::SpeculativeJIT::compileArithMod):
>+
> 2014-01-07  Filip Pizlo  <fpizlo@apple.com>
> 
>         Merge trunk r161465.
>Index: Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp	(revision 161438)
>+++ Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp	(working copy)
>@@ -3476,6 +3476,17 @@ void SpeculativeJIT::compileArithMod(Nod
>         GPRReg quotientThenRemainderGPR = quotientThenRemainder.gpr();
>         GPRReg multiplyAnswerGPR = multiplyAnswer.gpr();
> 
>+        JITCompiler::JumpList done;
>+        
>+        if (shouldCheckOverflow(node->arithMode()))
>+            speculationCheck(Overflow, JSValueRegs(), 0, m_jit.branchTest32(JITCompiler::Zero, divisorGPR));
>+        else {
>+            JITCompiler::Jump denominatorNotZero = m_jit.branchTest32(JITCompiler::NonZero, divisorGPR);
>+            m_jit.move(divisorGPR, quotientThenRemainderGPR);
>+            done.append(m_jit.jump());
>+            denominatorNotZero.link(&m_jit);
>+        }
>+
>         m_jit.assembler().sdiv(quotientThenRemainderGPR, dividendGPR, divisorGPR);
>         // FIXME: It seems like there are cases where we don't need this? What if we have
>         // arithMode() == Arith::Unchecked?
>@@ -3492,6 +3503,8 @@ void SpeculativeJIT::compileArithMod(Nod
>             numeratorPositive.link(&m_jit);
>         }
> 
>+        done.link(&m_jit);
>+        
>         int32Result(quotientThenRemainderGPR, node);
> #elif CPU(ARM64)
>         GPRTemporary temp(this);
>@@ -3502,6 +3515,17 @@ void SpeculativeJIT::compileArithMod(Nod
>         GPRReg quotientThenRemainderGPR = quotientThenRemainder.gpr();
>         GPRReg multiplyAnswerGPR = multiplyAnswer.gpr();
> 
>+        JITCompiler::JumpList done;
>+    
>+        if (shouldCheckOverflow(node->arithMode()))
>+            speculationCheck(Overflow, JSValueRegs(), 0, m_jit.branchTest32(JITCompiler::Zero, divisorGPR));
>+        else {
>+            JITCompiler::Jump denominatorNotZero = m_jit.branchTest32(JITCompiler::NonZero, divisorGPR);
>+            m_jit.move(divisorGPR, quotientThenRemainderGPR);
>+            done.append(m_jit.jump());
>+            denominatorNotZero.link(&m_jit);
>+        }
>+
>         m_jit.assembler().sdiv<32>(quotientThenRemainderGPR, dividendGPR, divisorGPR);
>         // FIXME: It seems like there are cases where we don't need this? What if we have
>         // arithMode() == Arith::Unchecked?
>@@ -3518,6 +3542,8 @@ void SpeculativeJIT::compileArithMod(Nod
>             numeratorPositive.link(&m_jit);
>         }
> 
>+        done.link(&m_jit);
>+
>         int32Result(quotientThenRemainderGPR, node);
> #else // not architecture that can do integer division
>         RELEASE_ASSERT_NOT_REACHED();

Flags:

ggaren: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 126612: 220583