ARM: Implement untagged input for TranscendentalCacheStub.

src/arm/code-stubs-arm.cc

 // Copyright 2011 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 3054 matching lines @@
   }
 }
 
 
 void TypeRecordingBinaryOpStub::GenerateRegisterArgsPush(MacroAssembler* masm) {
   __ Push(r1, r0);
 }
 
 
 void TranscendentalCacheStub::Generate(MacroAssembler* masm) {
-  // Argument is a number and is on stack and in r0.
-  Label runtime_call;
   Label input_not_smi;
   Label loaded;
+  Label runtime_call;
+  Label invalid_cache;
+  const Register scratch0 = r9;
+  const Register scratch1 = r10;

[Søren Thygesen Gjesse, 2011/03/02 10:09:45]

As discussed offline using r10 here is might be the cause of the crashes you are
currently facing.

[Karl Klose, 2011/03/02 11:16:41]

Yes, it is. Used different register.

+  const Register tagged_input = r7;
+  const bool tagged = (argument_type_ == TAGGED);
 
   if (CpuFeatures::IsSupported(VFP3)) {
-    // Load argument and check if it is a smi.
-    __ JumpIfNotSmi(r0, &input_not_smi);
+    CpuFeatures::Scope scope(VFP3);
+    if (tagged) {
+      // Argument is a number and is on stack and in r0.
+      // Load argument and check if it is a smi.
+      __ JumpIfNotSmi(r0, &input_not_smi);
 
-    CpuFeatures::Scope scope(VFP3);
-    // Input is a smi. Convert to double and load the low and high words
-    // of the double into r2, r3.
-    __ IntegerToDoubleConversionWithVFP3(r0, r3, r2);
-    __ b(&loaded);
+      CpuFeatures::Scope scope(VFP3);

[Søren Thygesen Gjesse, 2011/03/02 10:09:45]

Duplicate CpuFeatures::Scope.

[Karl Klose, 2011/03/02 11:16:41]

Done, removed.

+      // Input is a smi. Convert to double and load the low and high words
+      // of the double into r2, r3.
+      __ IntegerToDoubleConversionWithVFP3(r0, r3, r2);
+      __ b(&loaded);
 
-    __ bind(&input_not_smi);
-    // Check if input is a HeapNumber.
-    __ CheckMap(r0,
-                r1,
-                Heap::kHeapNumberMapRootIndex,
-                &runtime_call,
-                true);
-    // Input is a HeapNumber. Load it to a double register and store the
-    // low and high words into r2, r3.
-    __ Ldrd(r2, r3, FieldMemOperand(r0, HeapNumber::kValueOffset));
-
+      __ bind(&input_not_smi);
+      // Check if input is a HeapNumber.
+      __ CheckMap(r0,
+                  r1,
+                  Heap::kHeapNumberMapRootIndex,
+                  &runtime_call,
+                  true);
+      // Input is a HeapNumber. Load it to a double register and store the
+      // low and high words into r2, r3.
+      __ vldr(d0, FieldMemOperand(r0, HeapNumber::kValueOffset));
+      __ vmov(r2, r3, d0);
+    } else {
+      // Input is untagged double in d2. Output goes to d2.
+      __ vmov(r2, r3, d2);
+    }
     __ bind(&loaded);
     // r2 = low 32 bits of double value
     // r3 = high 32 bits of double value
     // Compute hash (the shifts are arithmetic):
     //   h = (low ^ high); h ^= h >> 16; h ^= h >> 8; h = h & (cacheSize - 1);
     __ eor(r1, r2, Operand(r3));
     __ eor(r1, r1, Operand(r1, ASR, 16));
     __ eor(r1, r1, Operand(r1, ASR, 8));
     ASSERT(IsPowerOf2(TranscendentalCache::kCacheSize));
     __ And(r1, r1, Operand(TranscendentalCache::kCacheSize - 1));
 
     // r2 = low 32 bits of double value.
     // r3 = high 32 bits of double value.
     // r1 = TranscendentalCache::hash(double value).
     __ mov(r0,
            Operand(ExternalReference::transcendental_cache_array_address()));
     // r0 points to cache array.
     __ ldr(r0, MemOperand(r0, type_ * sizeof(TranscendentalCache::caches_[0])));
     // r0 points to the cache for the type type_.
     // If NULL, the cache hasn't been initialized yet, so go through runtime.
     __ cmp(r0, Operand(0, RelocInfo::NONE));
-    __ b(eq, &runtime_call);
+    __ b(eq, &invalid_cache);
 
 #ifdef DEBUG
     // Check that the layout of cache elements match expectations.
     { TranscendentalCache::Element test_elem[2];
       char* elem_start = reinterpret_cast<char*>(&test_elem[0]);
       char* elem2_start = reinterpret_cast<char*>(&test_elem[1]);
       char* elem_in0 = reinterpret_cast<char*>(&(test_elem[0].in[0]));
       char* elem_in1 = reinterpret_cast<char*>(&(test_elem[0].in[1]));
       char* elem_out = reinterpret_cast<char*>(&(test_elem[0].output));
       CHECK_EQ(12, elem2_start - elem_start);  // Two uint_32's and a pointer.
       CHECK_EQ(0, elem_in0 - elem_start);
       CHECK_EQ(kIntSize, elem_in1 - elem_start);
       CHECK_EQ(2 * kIntSize, elem_out - elem_start);
     }
 #endif
 
     // Find the address of the r1'st entry in the cache, i.e., &r0[r1*12].
     __ add(r1, r1, Operand(r1, LSL, 1));
     __ add(r0, r0, Operand(r1, LSL, 2));
     // Check if cache matches: Double value is stored in uint32_t[2] array.
-    __ ldm(ia, r0, r4.bit()| r5.bit() | r6.bit());
+    __ ldm(ia, r0, r4.bit() | r5.bit() | r6.bit());
     __ cmp(r2, r4);
     __ b(ne, &runtime_call);
     __ cmp(r3, r5);
     __ b(ne, &runtime_call);
-    // Cache hit. Load result, pop argument and return.
-    __ mov(r0, Operand(r6));
-    __ pop();
+    // Cache hit. Load result, cleanup and return.
+    if (tagged) {
+      // Pop input value from stack and load result into r0.
+      __ pop();
+      __ mov(r0, Operand(r6));
+    } else {
+      //__ stop("cache hit");

[Søren Thygesen Gjesse, 2011/03/02 10:09:45]

Debug code.

[Karl Klose, 2011/03/02 11:16:41]

Done.

+      // Load result into d2.
+       __ vldr(d2, FieldMemOperand(r6, HeapNumber::kValueOffset));
+    }
+    __ Ret();
+  } // if (CpuFeatures::IsSupported(VFP3))

[Søren Thygesen Gjesse, 2011/03/02 10:09:45]

Two spaces before //.

[Karl Klose, 2011/03/02 11:16:41]

Done.

+
+  __ bind(&runtime_call);

[Søren Thygesen Gjesse, 2011/03/02 10:09:45]

Maybe the name of this label should be "calculate", as it does no necessarily
call runtime (normally C-runtime is not counted as runtime).

[Karl Klose, 2011/03/02 11:16:41]

Done.

+  if (tagged) {
+    __ bind(&invalid_cache);
+    __ TailCallExternalReference(ExternalReference(RuntimeFunction()), 1, 1);
+  } else {
+    if (!CpuFeatures::IsSupported(VFP3)) UNREACHABLE();
+    CpuFeatures::Scope scope(VFP3);
+
+    Register cache_entry = r0;
+    Register heap_number_map = r5;
+    Label no_update;
+    Label skip_cache;
+
+    // Call C function to calculate the result and update the cache.
+    __ push(r0);

[Søren Thygesen Gjesse, 2011/03/02 10:09:45]

r0 -> cache_entry

[Karl Klose, 2011/03/02 11:16:41]

Done.

+    __ push(lr);

[Søren Thygesen Gjesse, 2011/03/02 10:09:45]

The code for calling the C functions are repeated. Maybe refactor it into
TranscendentalCacheStub::GenerateCallCFunction.

[Karl Klose, 2011/03/02 11:16:41]

Done.

+    __ PrepareCallCFunction(2, scratch0);
+    __ vmov(r0, r1, d2);
+    switch(type_) {
+      case TranscendentalCache::SIN:
+        __ CallCFunction(ExternalReference::math_sin_double_function(), 2);
+        break;
+      case TranscendentalCache::COS:
+        __ CallCFunction(ExternalReference::math_cos_double_function(), 2);
+        break;
+      case TranscendentalCache::LOG:
+        __ CallCFunction(ExternalReference::math_log_double_function(), 2);
+        break;
+      default:
+        UNIMPLEMENTED();
+        break;
+    }
+    __ pop(lr);
+    __ GetCFunctionDoubleResult(d2);
+
+    // Try to update the cache.
+    __ pop(cache_entry);
+
+    // Check if the cache is valid.
+    __ cmp(r0, Operand(0, RelocInfo::NONE));
+    __ b(&no_update, eq);
+    __ LoadRoot(r5, Heap::kHeapNumberMapRootIndex);
+    __ AllocateHeapNumber(r6, scratch0, scratch1, r5, &no_update);
+    __ vstr(d2, FieldMemOperand(r6, HeapNumber::kValueOffset));
+    __ stm(ia, cache_entry, r2.bit() | r3.bit() | r6.bit());
+    __ bind(&no_update);
+    __ Ret();
+
+    __ bind(&skip_cache);
+    // Call C function to calculate the result and answer directly
+    // without updating the cache.
+    __ push(lr);
+    __ PrepareCallCFunction(2, scratch0);
+    __ vmov(r0, r1, d2);
+    switch(type_) {
+      case TranscendentalCache::SIN:
+        __ CallCFunction(ExternalReference::math_sin_double_function(), 2);
+        break;
+      case TranscendentalCache::COS:
+        __ CallCFunction(ExternalReference::math_cos_double_function(), 2);
+        break;
+      case TranscendentalCache::LOG:
+        __ CallCFunction(ExternalReference::math_log_double_function(), 2);
+        break;
+      default:
+        UNIMPLEMENTED();
+        break;
+    }
+    __ pop(lr);
+    __ GetCFunctionDoubleResult(d2);
+    // We return the value in d2 without adding it to the cache, but
+    // we cause a scavenging GC so that future allocations will succeed.
+    __ EnterInternalFrame();
+    // Allocate an unused object bigger than a HeapNumber.
+    __ mov(scratch0, Operand(Smi::FromInt(2 * kDoubleSize)));

[Søren Thygesen Gjesse, 2011/03/02 10:09:45]

Shouldn't HeapNumber::kSize be enough? Allocating a heap number has already
failed.

[Karl Klose, 2011/03/02 11:16:41]

Done.

+    __ push(scratch0);
+    __ CallRuntimeSaveDoubles(Runtime::kAllocateInNewSpace);
+    __ LeaveInternalFrame();
+    __ Ret();
+
+    __ bind(&invalid_cache);
+    // The cache is invalid. Call runtime which will recreate the
+    // cache.
+    __ LoadRoot(r5, Heap::kHeapNumberMapRootIndex);
+    __ AllocateHeapNumber(r6, scratch0, scratch1, r5, &skip_cache);

[Søren Thygesen Gjesse, 2011/03/02 10:09:45]

Any reason for having the skip cache code before this? Jumping forwards in this
case makes easier to read the code (I guess).

[Karl Klose, 2011/03/02 11:16:41]

There is no reason. I changed the order.

+    __ vstr(d2, FieldMemOperand(r6, HeapNumber::kValueOffset));
+    __ EnterInternalFrame();
+    __ push(r6);
+    __ CallRuntime(RuntimeFunction(), 1);
+    __ LeaveInternalFrame();
+    __ vldr(d2, FieldMemOperand(r0, HeapNumber::kValueOffset));
     __ Ret();
   }
-
-  __ bind(&runtime_call);
-  __ TailCallExternalReference(ExternalReference(RuntimeFunction()), 1, 1);
 }
 
 
 Runtime::FunctionId TranscendentalCacheStub::RuntimeFunction() {
   switch (type_) {
     // Add more cases when necessary.
     case TranscendentalCache::SIN: return Runtime::kMath_sin;
     case TranscendentalCache::COS: return Runtime::kMath_cos;
     case TranscendentalCache::LOG: return Runtime::kMath_log;
     default:
@@ skipping 2964 matching lines @@
   __ strb(untagged_value, MemOperand(external_pointer, untagged_key));
   __ Ret();
 }
 
 
 #undef __
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_ARM