[webkit-reviews] review granted: [Bug 131205] LLInt interpreter code should be generated as part of one function : [Attachment 229066] Patch addressing issues raised in review
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Apr 10 14:58:05 PDT 2014
Mark Lam <mark.lam at apple.com> has granted Michael Saboff <msaboff at apple.com>'s
request for review:
Bug 131205: LLInt interpreter code should be generated as part of one function
https://bugs.webkit.org/show_bug.cgi?id=131205
Attachment 229066: Patch addressing issues raised in review
https://bugs.webkit.org/attachment.cgi?id=229066&action=review
------- Additional Comments from Mark Lam <mark.lam at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=229066&action=review
r=me with fixes.
> Source/JavaScriptCore/ChangeLog:40
> + as well as including files from other than the local directory via
a newly
Extra space between "as including".
> Source/JavaScriptCore/ChangeLog:45
> + Updated the generationo f the masm only .sym file to be written once
at the end
/generationo f/generation of/.
> Source/JavaScriptCore/ChangeLog:46
> + of online assembling.
/online assembling/offline assembly/.
> Source/JavaScriptCore/llint/LLIntData.h:38
> +const int opcodeMapSize = NUMBER_OF_BYTECODE_IDS +
NUMBER_OF_CLOOP_BYTECODE_HELPER_IDS + NUMBER_OF_BYTECODE_HELPER_IDS;
This looks identical to the definition of numOpcodeIDs. I think once upon a
time, these 2 values are different, but now they are redundant. Let's get rid
of opcodeMapSize and just use numOpcodeIDs everywhere instead.
> Source/JavaScriptCore/llint/LLIntData.h:41
> +const int opcodeMapSize = NUMBER_OF_BYTECODE_IDS +
NUMBER_OF_BYTECODE_HELPER_IDS;
Ditto.
> Source/JavaScriptCore/llint/LLIntData.h:64
> + friend void* getCodePtr(unsigned);
> + friend void* getHelperCodePtr(unsigned);
I think these 2 are now obsolete and not referenced anywhere. Please remove.
> Source/JavaScriptCore/llint/LLIntData.h:118
> ALWAYS_INLINE void* getCodePtr(JSC::EncodedJSValue glueHelper())
I presume this is still needed for "LLInt::getCodePtr(getHostCallReturnValue))"
or something. Please confirm that it is still needed. If not, please remove.
> Source/JavaScriptCore/llint/LowLevelInterpreter.h:50
> #endif // !ENABLE(LLINT_C_LOOP)
/!ENABLE(LLINT_C_LOOP)/ENABLE(LLINT_C_LOOP)/ since there's no longer a
!ENABLE(LLINT_C_LOOP) case.
More information about the webkit-reviews
mailing list