[webkit-reviews] review granted: [Bug 75812] JSC should be a triple-tier VM : [Attachment 127001] the patch

[bugzilla-daemon at webkit.org]

Gavin Barraclough <barraclough at apple.com> has granted Filip Pizlo
<fpizlo at apple.com>'s request for review:
Bug 75812: JSC should be a triple-tier VM
https://bugs.webkit.org/show_bug.cgi?id=75812

Attachment 127001: the patch
https://bugs.webkit.org/attachment.cgi?id=127001&action=review

------- Additional Comments from Gavin Barraclough <barraclough at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=127001&action=review


Okay, this basically all looks good.  Couple small of issues:

Please revert JSActivation.h & Heap.cpp (whitespace only changes).

Please rename Interpreter::enabled() (to make it clear which interpreter this
is enabling - e.g. 'classicEnabled()' would be fine.)

Per our discussion, I think 'Helpers' isn't a great name, since it isn't
descriptive of what it helps.  I'm happy with your suggested 'SlowPaths'.

It's a shame that LLIntOffsetsExtractor is LLInt specific – it seems this
pattern could be generalized to provide offsets into classes for use by the
JITs, too, and reduce the friendliness of the codebase.  But I'm not asking for
this change in this patch, it's fine as is.

> Source/JavaScriptCore/llint/LowLevelInterpreter.asm:70
> +# These declarations must match interpreter/RegisterFile.h.

Are these constants defended anywhere by compile time ASSERTs?