LGTM
I am thinking whether updating the cache with heap number objects in the
non-tagged case defeats the purpose of performing double operations using
double
registers and not heap number objects.
Maybe we should experiment with having a transcendental cache which can
store
both heap number objects and raw double values, or ignore the cache in the
non-tagged case.
http://codereview.chromium.org/6591073/diff/1/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):
http://codereview.chromium.org/6591073/diff/1/src/arm/code-stubs-arm.cc#newcode3080
src/arm/code-stubs-arm.cc:3080: const Register scratch1 = r10;
As discussed offline using r10 here is might be the cause of the crashes
you are currently facing.
http://codereview.chromium.org/6591073/diff/1/src/arm/code-stubs-arm.cc#newcode3091
src/arm/code-stubs-arm.cc:3091: CpuFeatures::Scope scope(VFP3);
Duplicate CpuFeatures::Scope.
http://codereview.chromium.org/6591073/diff/1/src/arm/code-stubs-arm.cc#newcode3165
src/arm/code-stubs-arm.cc:3165: //__ stop("cache hit");
Debug code.
http://codereview.chromium.org/6591073/diff/1/src/arm/code-stubs-arm.cc#newcode3170
src/arm/code-stubs-arm.cc:3170: } // if (CpuFeatures::IsSupported(VFP3))
Two spaces before //.
http://codereview.chromium.org/6591073/diff/1/src/arm/code-stubs-arm.cc#newcode3172
src/arm/code-stubs-arm.cc:3172: __ bind(&runtime_call);
Maybe the name of this label should be "calculate", as it does no
necessarily call runtime (normally C-runtime is not counted as runtime).
http://codereview.chromium.org/6591073/diff/1/src/arm/code-stubs-arm.cc#newcode3186
src/arm/code-stubs-arm.cc:3186: __ push(r0);
r0 -> cache_entry
http://codereview.chromium.org/6591073/diff/1/src/arm/code-stubs-arm.cc#newcode3187
src/arm/code-stubs-arm.cc:3187: __ push(lr);
The code for calling the C functions are repeated. Maybe refactor it
into TranscendentalCacheStub::GenerateCallCFunction.
http://codereview.chromium.org/6591073/diff/1/src/arm/code-stubs-arm.cc#newcode3246
src/arm/code-stubs-arm.cc:3246: __ mov(scratch0, Operand(Smi::FromInt(2
* kDoubleSize)));
Shouldn't HeapNumber::kSize be enough? Allocating a heap number has
already failed.
http://codereview.chromium.org/6591073/diff/1/src/arm/code-stubs-arm.cc#newcode3256
src/arm/code-stubs-arm.cc:3256: __ AllocateHeapNumber(r6, scratch0,
scratch1, r5, &skip_cache);
Any reason for having the skip cache code before this? Jumping forwards
in this case makes easier to read the code (I guess).
http://codereview.chromium.org/6591073/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
