[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





--- Comment #5 from Filip Pizlo <fpizlo at apple.com>  2012-08-23 18:52:11 PST ---
(In reply to comment #3)
> Thanks Geoffrey
> 
> (In reply to comment #2)
> > (From update of attachment 160089 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=160089&action=review
> > 
> > > Source/JavaScriptCore/dfg/DFGOperations.h:183
> > >  double DFG_OPERATION operationFModOnInts(int32_t, int32_t) WTF_INTERNAL;
> > 
> > You should remove this function, since you're replacing it.
> > 
> > > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp:2545
> > > +    speculationCheck(Overflow, JSValueRegs(), NoNode, checkZero);
> > 
> > Did you verify what happens in the recovery path when this speculation fails?
> Could let me know more specific what "this speculation fails" means?
> Do you intend to mention about speculationCheck() itself or checking speculation of the divisor?

I think he means both.  It appears that you have not tested this case.  I think that just running layout tests should be enough - we should have excellent coverage over corner cases of integer modulo.

That said, your code will pretty obviously cause badness (the speculationCheck() is invoked in the wrong place, and you're only checking for one of the corner cases, not all of them) so if layout tests do pass by some fluke, then you should add a new layout test or file a bug against Tools/Tests for the test to be improved.

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