Bug 99258 - Array length array profiling is broken in the baseline JIT
Summary: Array length array profiling is broken in the baseline JIT
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-13 20:14 PDT by Filip Pizlo
Modified: 2012-10-14 19:31 PDT (History)
5 users (show)

See Also:


Attachments
the patch (1.77 KB, patch)
2012-10-13 20:21 PDT, Filip Pizlo
oliver: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2012-10-13 20:14:38 PDT
It seems that a well-intentioned no-DFG-JIT build fix borked it.
Comment 1 Filip Pizlo 2012-10-13 20:21:09 PDT
Created attachment 168573 [details]
the patch
Comment 2 Filip Pizlo 2012-10-14 12:43:36 PDT
Landed in http://trac.webkit.org/changeset/131268
Comment 3 Geoffrey Garen 2012-10-14 19:24:57 PDT
Comment on attachment 168573 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=168573&action=review

> Source/JavaScriptCore/ChangeLog:11
> +        canBeOptimized() returns true. But m_canBeOptimized is only initialized during
> +        full method compiles, so in a stub compile it may (or may not) be false, meaning

Please also fix the JIT constructor, so that it default-initializes m_canBeOptimized to false.

Why is this initialization code in privateCompile(), and not in the the constructor, to begin with?
Comment 4 Filip Pizlo 2012-10-14 19:31:16 PDT
(In reply to comment #3)
> (From update of attachment 168573 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=168573&action=review
> 
> > Source/JavaScriptCore/ChangeLog:11
> > +        canBeOptimized() returns true. But m_canBeOptimized is only initialized during
> > +        full method compiles, so in a stub compile it may (or may not) be false, meaning
> 
> Please also fix the JIT constructor, so that it default-initializes m_canBeOptimized to false.

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

> 
> Why is this initialization code in privateCompile(), and not in the the constructor, to begin with?

I think that it makes sense for the decision logic to be in privateCompile().  In particular, this statement:

    DFG::CapabilityLevel level = m_codeBlock->canCompileWithDFG();

does an O(n) walk over the code block's bytecode.  We don't want that to happen every time we construct a JIT, since we do that for every stub compile.