[Webkit-unassigned] [Bug 67985] SpeculativeJIT::shouldSpeculateInteger(NodeIndex, NodeIndex) should return false if either node can be double

[bugzilla-daemon at webkit.org]

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





--- Comment #7 from Filip Pizlo <fpizlo at apple.com>  2011-09-13 13:44:20 PST ---
(In reply to comment #6)
> (From update of attachment 107217 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=107217&action=review
> 
> r=me but please don't check in the accidental change to Platform.h.

Ooops!

> 
> > Source/JavaScriptCore/ChangeLog:38
> > +        * dfg/DFGGenerationInfo.h:
> > +        (JSC::DFG::isJSFormat):
> > +        (JSC::DFG::isJSInteger):
> > +        (JSC::DFG::isJSDouble):
> > +        (JSC::DFG::isJSCell):
> > +        (JSC::DFG::isJSBoolean):
> > +        (JSC::DFG::GenerationInfo::isJSFormat):
> > +        (JSC::DFG::GenerationInfo::isJSInteger):
> > +        (JSC::DFG::GenerationInfo::isJSDouble):
> > +        (JSC::DFG::GenerationInfo::isJSCell):
> > +        (JSC::DFG::GenerationInfo::isJSBoolean):
> > +        * dfg/DFGJITCodeGenerator.cpp:
> > +        (JSC::DFG::JITCodeGenerator::isKnownInteger):
> > +        (JSC::DFG::JITCodeGenerator::isKnownNumeric):
> > +        (JSC::DFG::JITCodeGenerator::isKnownCell):
> > +        (JSC::DFG::JITCodeGenerator::isKnownNotInteger):
> > +        (JSC::DFG::JITCodeGenerator::isKnownBoolean):
> > +        * dfg/DFGJITCodeGenerator.h:
> > +        * dfg/DFGNonSpeculativeJIT.cpp:
> > +        (JSC::DFG::NonSpeculativeJIT::compile):
> > +        * dfg/DFGSpeculativeJIT.h:
> > +        (JSC::DFG::SpeculativeJIT::isInteger):
> > +        (JSC::DFG::SpeculativeJIT::shouldSpeculateDouble):
> > +        (JSC::DFG::SpeculativeJIT::shouldNotSpeculateInteger):
> > +        (JSC::DFG::SpeculativeJIT::shouldSpeculateInteger):
> 
> It's great to put line-by-line comments next to this auto-generated helper text, but in cases where you don't do that, I think it's better just to delete the text.

OK - I'll switch this to putting comments next to the auto-generated text.

> > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h:400
> > +        return !(shouldNotSpeculateInteger(op1) || shouldNotSpeculateInteger(op2)) && (shouldSpeculateInteger(op1) || shouldSpeculateInteger(op2));
> 
> The fight between shouldNotSpeculateInteger() and shouldSpeculateInteger() is a bit mind-bending, but I don't have a specific suggestion to make right now.
> 
> > Source/JavaScriptCore/wtf/Platform.h:968
> > -#define ENABLE_TIERED_COMPILATION 0
> > +#define ENABLE_TIERED_COMPILATION 1
> 
> Hey man, if you want to enable it so badly, why don't you just submit a patch? ;)

Turns out that this patch is probably a prerequisite for enabling it, but it may not be sufficient.  In browser, we're seeing somewhat different speculations with awkward behavior as a result.  This patch will fix part, but not all, of that.

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