On 2015/08/25 18:30:01, Yang wrote:
On 2015/08/25 18:12:09, Michael Starzinger wrote:
> Essentially one high-level comment/suggestion. Happy to discuss in
person
and/or
> be convinced otherwise. Would also be interested in what Benedikt thinks
about
> this.
>
> https://codereview.chromium.org/1306993003/diff/1/src/ast.h
> File src/ast.h (right):
>
> https://codereview.chromium.org/1306993003/diff/1/src/ast.h#newcode2052
> src/ast.h:2052: const Runtime::Function* function_;
> If below comment is addressed then we would need two fields here again,
either
> Runtime::Function or Context::Index. Still a memory saving from what was
here
> before, and if size really becomes the argument here, then there are
(other)
> ways to compress it down I think.
>
> https://codereview.chromium.org/1306993003/diff/1/src/runtime/runtime.h
> File src/runtime/runtime.h (right):
>
>
https://codereview.chromium.org/1306993003/diff/1/src/runtime/runtime.h#newcode995
> src/runtime/runtime.h:995: enum IntrinsicType { RUNTIME, INLINE,
CONTEXT };
> High-level feedback: I am not sure whether overloading the
Runtime::Function
> table like this is the way to go. Basically what we need is a mapping
(name
->
> context-index) IIUC. The other fields (e.g. nargs, result_size) make no
sense
> for "imported context functions". Also them being implemented in
JavaScript
as
> opposed to C++ makes them different. Would it be worth it to have a
separate
> lookup functionality outside of the Runtime class?
I'm somewhat torn about this as well, and initially actually had the
lookup
implemented separately. However, the separate implementation would share
a lot
of the same code, and, if done via hash table lookup, use the same data
structure as well. Furthermore, the difference between C++ runtime
functions
to
JS builtin functions is no larger than the difference between C++ runtime
functions and inlined %_ functions. Common to all of them is that they
can be
addressed via % syntax. So I think grouping the three of them together is
alright.
But I can be convince otherwise, if you feel strongly about this.
I separated the context slot lookup. And ported to all platforms. Please
review.
https://codereview.chromium.org/1306993003/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
