[webkit-reviews] review requested: [Bug 67176] JavaScriptCore does not have tiered compilation : [Attachment 106369] the patch - partial fix review

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Sep 5 17:49:30 PDT 2011


Filip Pizlo <fpizlo at apple.com> has asked  for review:
Bug 67176: JavaScriptCore does not have tiered compilation
https://bugs.webkit.org/show_bug.cgi?id=67176

Attachment 106369: the patch - partial fix review
https://bugs.webkit.org/attachment.cgi?id=106369&action=review

------- Additional Comments from Filip Pizlo <fpizlo at apple.com>
For the more subtle parts of the review I proceeded as follows.  For the last
two bullets, I kept the code as-is, because AFAIK that's the lesser of the
various evils right now.

branchAdd32 should use scratchRegister not r11:
    I renamed it to branchAdd32Absolute and moved it to the subclasses.

DynamicPrediction:
    I renamed DynamicPrediction to StrongPrediction and StaticPrediction to
WeakPrediction.

setting m_fooBarCodeBlock before we know that optimizing will succeed:
    I kept this as-is, because CodeBlock is not ref-counted, and the
CodeBlock::m_alternative pointer is an OwnPtr that needs to be initialized
prior to compiling.  So we need a releaseAlternative() method, and we need to
call this method in Executable.cpp in a number of places, if we want to back
out of optimization.  We can either (1) make CodeBlock ref-counted just to
satisfy this case and get rid of releaseAlternative(), or (2) keep it as-is.

JITType enum:
    I kept this as-is, because currently having two enums (policy and
mechanism) would just mean having two enums that say the same thing.


More information about the webkit-reviews mailing list