Almost LGTM, let's do last round and I'll land it
http://codereview.chromium.org/6462029/diff/1/src/arm/simulator-arm.cc
File src/arm/simulator-arm.cc (right):
http://codereview.chromium.org/6462029/diff/1/src/arm/simulator-arm.cc#newcode1534
src/arm/simulator-arm.cc:1534: typedef v8::Handle<v8::Value>
(*SimulatorRuntimeDirectApiCall)(int32_t arg0);
On 2011/02/21 10:25:35, Zaheer wrote:
On 2011/02/17 16:13:44, antonm wrote:
> Do we need word Direct here? It looks like simulator doesn't have
to know
it's
> anything direct. But if you feel it conveys important semantic
thing, of
course
> we should keep it.
I think direct differentiates from the builtin call and i would like
to keep it
for now.
ok
http://codereview.chromium.org/6462029/diff/1/src/arm/stub-cache-arm.cc
File src/arm/stub-cache-arm.cc (right):
http://codereview.chromium.org/6462029/diff/1/src/arm/stub-cache-arm.cc#newcode1245
src/arm/stub-cache-arm.cc:1245: ASSERT(holder_reg.is(receiver) ||
holder_reg.is(scratch1));
On 2011/02/21 10:25:35, Zaheer wrote:
On 2011/02/17 16:13:44, antonm wrote:
> do you waht to assert this or rather the fact that
!holder_reg.is(scratch{2,3})?
The holder_reg.is(scratch{2,3}) check already happens in
checkprototypes.
So why you bother asserting here? What kind of information this assert
conveys to the reader? Sorry, I am probably missing something obvious
here.
http://codereview.chromium.org/6462029/diff/1/src/arm/stub-cache-arm.cc#newcode1268
src/arm/stub-cache-arm.cc:1268: __ str(scratch2, MemOperand(sp, 1 *
kPointerSize));
On 2011/02/21 10:25:35, Zaheer wrote:
On 2011/02/17 16:13:44, antonm wrote:
> Please, explain briefly why we need additional wrapping here.
This to create internal::Object** (AccessorInfo only member)
>
> BTW, is this ABI universal across ARM compilers?
I cant see any ABI/compiler specific code here. May be i miss your
point.
I am sorry, I thought that's some additional wrapping.
http://codereview.chromium.org/6462029/diff/8001/src/arm/stub-cache-arm.cc#newcode1247
src/arm/stub-cache-arm.cc:1247: // Push AccessorInfo arguments and
property name below the
won't it be more correct to phrase it like: build AccessorInfo::args_
list on the stack and push property name below... ?
http://codereview.chromium.org/6462029/diff/8001/src/arm/stub-cache-arm.cc#newcode1250
src/arm/stub-cache-arm.cc:1250: __ mov(scratch2, sp);
maybe: // scratch = AccessorInfo::args_ ?
http://codereview.chromium.org/6462029/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
