Move (uppercase) JS builtins from js builtins object to native context.

Move (uppercase) JS builtins from js builtins object to native context.

R=bmeurer@chromium.org, mstarzinger@chromium.org, rmcilroy@chromium.org

Committed: https://crrev.com/b42c4459e64bb8afff40917ed5df1cd1edc6642b
Cr-Commit-Position: refs/heads/master@{#30402}

[Yang, 2015-08-26 13:46:08]

Can I get a round of comments before porting?

[Michael Starzinger, 2015-08-26 13:56:35]

I would be fine with going for this approach. Looking good.

[rmcilroy, 2015-08-26 14:07:57]

Interpreter changes look good to me.

[Michael Starzinger, 2015-08-26 14:16:42]

LGTM modulo architecture ports. Didn't look at interpreter though.

https://codereview.chromium.org/1316943002/diff/1/src/compiler/ast-graph-buil...
File src/compiler/ast-graph-builder.cc (right):

https://codereview.chromium.org/1316943002/diff/1/src/compiler/ast-graph-buil...
src/compiler/ast-graph-builder.cc:2544: 
nit: Let's drop this empty newline, the pushes belong to the comment above.

https://codereview.chromium.org/1316943002/diff/1/src/contexts.h
File src/contexts.h (right):

https://codereview.chromium.org/1316943002/diff/1/src/contexts.h#newcode206
src/contexts.h:206: NATIVE_CONTEXT_JS_BUILTINS(V)
Is there a reason we are adding this to the NATIVE_CONTEXT_IMPORTED_FIELDS
instead of the NATIVE_CONTEXT_FIELDS below?

[Yang, 2015-08-26 14:18:52]

On 2015/08/26 14:16:42, Michael Starzinger wrote:
> LGTM modulo architecture ports. Didn't look at interpreter though.
> 
>
https://codereview.chromium.org/1316943002/diff/1/src/compiler/ast-graph-buil...
> File src/compiler/ast-graph-builder.cc (right):
> 
>
https://codereview.chromium.org/1316943002/diff/1/src/compiler/ast-graph-buil...
> src/compiler/ast-graph-builder.cc:2544: 
> nit: Let's drop this empty newline, the pushes belong to the comment above.
> 
> https://codereview.chromium.org/1316943002/diff/1/src/contexts.h
> File src/contexts.h (right):
> 
> https://codereview.chromium.org/1316943002/diff/1/src/contexts.h#newcode206
> src/contexts.h:206: NATIVE_CONTEXT_JS_BUILTINS(V)
> Is there a reason we are adding this to the NATIVE_CONTEXT_IMPORTED_FIELDS
> instead of the NATIVE_CONTEXT_FIELDS below?

yes. Im using %InstallToContext in runtime.js to put them on the native context,
and it does the name lookup for imported fields.

[Michael Starzinger, 2015-08-26 14:23:15]

On 2015/08/26 14:18:52, Yang wrote:
> On 2015/08/26 14:16:42, Michael Starzinger wrote:
> > LGTM modulo architecture ports. Didn't look at interpreter though.
> > 
> >
>
https://codereview.chromium.org/1316943002/diff/1/src/compiler/ast-graph-buil...
> > File src/compiler/ast-graph-builder.cc (right):
> > 
> >
>
https://codereview.chromium.org/1316943002/diff/1/src/compiler/ast-graph-buil...
> > src/compiler/ast-graph-builder.cc:2544: 
> > nit: Let's drop this empty newline, the pushes belong to the comment above.
> > 
> > https://codereview.chromium.org/1316943002/diff/1/src/contexts.h
> > File src/contexts.h (right):
> > 
> > https://codereview.chromium.org/1316943002/diff/1/src/contexts.h#newcode206
> > src/contexts.h:206: NATIVE_CONTEXT_JS_BUILTINS(V)
> > Is there a reason we are adding this to the NATIVE_CONTEXT_IMPORTED_FIELDS
> > instead of the NATIVE_CONTEXT_FIELDS below?
> 
> yes. Im using %InstallToContext in runtime.js to put them on the native
context,
> and it does the name lookup for imported fields.

Hmm, we could add it to Context::ImportedFieldIndexForName explicitly to avoid
surprises. WDYT? I am fine either way, just a suggestion.

[Benedikt Meurer, 2015-08-27 04:45:52]

LGTM once ports are done.

[rmcilroy, 2015-08-27 08:37:16]

Interpreter LGTM with one nit.

https://codereview.chromium.org/1316943002/diff/1/test/unittests/compiler/int...
File test/unittests/compiler/interpreter-assembler-unittest.cc (right):

https://codereview.chromium.org/1316943002/diff/1/test/unittests/compiler/int...
test/unittests/compiler/interpreter-assembler-unittest.cc:276:
IsIntPtrConstant(Context::SlotOffset(22))));
nit - could you add a test for the new m.LoadContextSlot(Node* context, int
slot_index) method you added here please.

[Yang, 2015-08-27 09:02:27]

Added platform ports and unittest for interpreter assembler.

Do you guys want to look at this again?

[Yang, 2015-08-27 09:02:33]

The CQ bit was checked by yangguo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-27 09:02:42]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1316943002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1316943002/20001

[Yang, 2015-08-27 09:04:21]

The CQ bit was checked by yangguo@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-08-27 09:04:32]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1316943002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1316943002/40001

[commit-bot: I haz the power, 2015-08-27 10:09:36]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-08-27 10:09:36]

Dry run: This issue passed the CQ dry run.

[Yang, 2015-08-27 10:16:30]

The CQ bit was checked by yangguo@chromium.org

[Yang, 2015-08-27 10:16:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
bmeurer@chromium.org, mstarzinger@chromium.org, rmcilroy@chromium.org
Link to the patchset: https://codereview.chromium.org/1316943002/#ps40001
(title: "remove Isolate::js_builtins_object")

[commit-bot: I haz the power, 2015-08-27 10:16:41]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1316943002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1316943002/40001

[commit-bot: I haz the power, 2015-08-27 10:18:29]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2015-08-27 10:18:57]

Patchset 3 (id:??) landed as
https://crrev.com/b42c4459e64bb8afff40917ed5df1cd1edc6642b
Cr-Commit-Position: refs/heads/master@{#30402}