[webkit-reviews] review granted: [Bug 216685] Move some LLInt globals into JSC::Config. : [Attachment 409213] proposed patch.

[bugzilla-daemon at webkit.org]

Keith Miller <keith_miller at apple.com> has granted Mark Lam
<mark.lam at apple.com>'s request for review:
Bug 216685: Move some LLInt globals into JSC::Config.
https://bugs.webkit.org/show_bug.cgi?id=216685

Attachment 409213: proposed patch.

https://bugs.webkit.org/attachment.cgi?id=409213&action=review




--- Comment #24 from Keith Miller <keith_miller at apple.com> ---
Comment on attachment 409213
  --> https://bugs.webkit.org/attachment.cgi?id=409213
proposed patch.

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

r=me with nits

> Source/JavaScriptCore/ChangeLog:18
> +	   2. Add a LLIntInitializeAssertScope to be used in
LLInt::initialize() to ensure
> +	      that it is only called during primodial initialization time.

It seems like this is unnecessary, since we will crash once the data has been
made read only anyway, right?

> Source/JavaScriptCore/llint/LLIntData.cpp:65
> +    LLIntInitializeAssertScope assertScope;

Nit: if you want to keep this assert, make it a ScopeExit, you're only using it
once anyway.

> Source/JavaScriptCore/offlineasm/arm.rb:269
> +			   newList << Instruction.new(codeOrigin, "move",
[Immediate.new(node.codeOrigin, labelRef.offset), tmp])
> +			   newList << Instruction.new(codeOrigin, "addp", [tmp,
node.operands[1]])

Can you put a FIXME to implement the logic to figure out which immediates are
encodable inline here?