[Webkit-unassigned] [Bug 94779] To reduce cost of converting type during modulo for integers

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=94779


Filip Pizlo <fpizlo at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #160089|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #4 from Filip Pizlo <fpizlo at apple.com>  2012-08-23 18:43:50 PST ---
(From update of attachment 160089)
View in context: https://bugs.webkit.org/attachment.cgi?id=160089&action=review

This patch will cause a regression on fast/js/integer-division-neg2tothe32-by-neg1.html.  Did you run that test?

Please make sure your code covers all of the corner cases where integer division fails, rather than just the obvious one (zero denominator).

> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:2539
> +    JITCompiler::Jump checkZero = m_jit.branch32(JITCompiler::Equal, op2GPR, TrustedImm32(0));

Why are you doing it this way?  Why not m_jit.branchTest32(JITCompiler::Zero, op2GPR)?

>> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:2545
>> +    speculationCheck(Overflow, JSValueRegs(), NoNode, checkZero);
> 
> Did you verify what happens in the recovery path when this speculation fails?

This is completely wrong.  Why are you emitting a speculationCheck *after* the call?  The speculationCheck() should be exactly where the branch is.  Please hoist this up to the checkZero, above.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.