Use IC register definitions in platform files.

src/ia32/ic-ia32.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/v8.h"
 
 #if V8_TARGET_ARCH_IA32
 
 #include "src/codegen.h"
 #include "src/ic-inl.h"
@@ skipping 370 matching lines @@
                       FixedArray::kHeaderSize);
 }
 
 
 void KeyedLoadIC::GenerateGeneric(MacroAssembler* masm) {
   // ----------- S t a t e -------------
   //  -- ecx    : key
   //  -- edx    : receiver
   //  -- esp[0] : return address
   // -----------------------------------
+  ASSERT(edx.is(ReceiverRegister()));
+  ASSERT(ecx.is(NameRegister()));
   Label slow, check_name, index_smi, index_name, property_array_property;
   Label probe_dictionary, check_number_dictionary;
 
   // Check that the key is a smi.
   __ JumpIfNotSmi(ecx, &check_name);
   __ bind(&index_smi);
   // Now the key is known to be a smi. This place is also jumped to from
   // where a numeric string is converted to a smi.
 
   GenerateKeyedLoadReceiverCheck(
@@ skipping 158 matching lines @@
   __ jmp(&index_smi);
 }
 
 
 void KeyedLoadIC::GenerateString(MacroAssembler* masm) {
   // ----------- S t a t e -------------
   //  -- ecx    : key (index)
   //  -- edx    : receiver
   //  -- esp[0] : return address
   // -----------------------------------
+  ASSERT(edx.is(ReceiverRegister()));
+  ASSERT(ecx.is(NameRegister()));
   Label miss;
 
   Register receiver = edx;
   Register index = ecx;
   Register scratch = ebx;
   Register result = eax;
 
   StringCharAtGenerator char_at_generator(receiver,
                                           index,
                                           scratch,
@@ skipping 12 matching lines @@
   GenerateMiss(masm);
 }
 
 
 void KeyedLoadIC::GenerateIndexedInterceptor(MacroAssembler* masm) {
   // ----------- S t a t e -------------
   //  -- ecx    : key
   //  -- edx    : receiver
   //  -- esp[0] : return address
   // -----------------------------------
+  ASSERT(edx.is(ReceiverRegister()));
+  ASSERT(ecx.is(NameRegister()));
   Label slow;
 
   // Check that the receiver isn't a smi.
   __ JumpIfSmi(edx, &slow);
 
   // Check that the key is an array index, that is Uint32.
   __ test(ecx, Immediate(kSmiTagMask | kSmiSignMask));
   __ j(not_zero, &slow);
 
   // Get the map of the receiver.
@@ skipping 22 matching lines @@
   GenerateMiss(masm);
 }
 
 
 void KeyedLoadIC::GenerateSloppyArguments(MacroAssembler* masm) {
   // ----------- S t a t e -------------
   //  -- ecx    : key
   //  -- edx    : receiver
   //  -- esp[0] : return address
   // -----------------------------------
+  ASSERT(edx.is(ReceiverRegister()));
+  ASSERT(ecx.is(NameRegister()));
   Label slow, notin;
   Factory* factory = masm->isolate()->factory();
   Operand mapped_location =
       GenerateMappedArgumentsLookup(masm, edx, ecx, ebx, eax, &notin, &slow);
   __ mov(eax, mapped_location);
   __ Ret();
   __ bind(&notin);
   // The unmapped lookup expects that the parameter map is in ebx.
   Operand unmapped_location =
       GenerateUnmappedArgumentsLookup(masm, ecx, ebx, eax, &slow);
@@ skipping 270 matching lines @@
                                   &slow, kDontCheckMap, kIncrementLength);
 }
 
 
 void LoadIC::GenerateMegamorphic(MacroAssembler* masm) {
   // ----------- S t a t e -------------
   //  -- ecx    : name
   //  -- edx    : receiver
   //  -- esp[0] : return address
   // -----------------------------------
+  ASSERT(edx.is(ReceiverRegister()));
+  ASSERT(ecx.is(NameRegister()));
 
   // Probe the stub cache.
   Code::Flags flags = Code::ComputeHandlerFlags(Code::LOAD_IC);
   masm->isolate()->stub_cache()->GenerateProbe(
       masm, flags, edx, ecx, ebx, eax);
 
   // Cache miss: Jump to runtime.
   GenerateMiss(masm);
 }
 
 
 void LoadIC::GenerateNormal(MacroAssembler* masm) {
   // ----------- S t a t e -------------
   //  -- ecx    : name
   //  -- edx    : receiver
   //  -- esp[0] : return address
   // -----------------------------------
+  ASSERT(edx.is(ReceiverRegister()));

[Jakob Kummerow, 2014/06/25 10:59:12]

I'm not too happy with these ASSERTs, as opposed to using ReceiverRegister()
instead of raw "edx" below. But I concede that it's hard to decide where to draw
the line. I can live with the ASSERT, at least for now.

[mvstanton, 2014/06/25 15:10:27]

Okay, yep. I'm cautious because when code starts mentioned 4, 5 or 6 registers 
within a few lines I feel like the reader is best served by knowing what those
registers are. It's extra cognitive load to remember "which one is
ReceiverRegister()? Oh yeah." That is, the reader will actually turn it back
into a register.

Plus when you consider that one of the IC spec registers plays another role
(say, it's the accumulator and a call is made), then you really have to be
careful against obscuring the plain old register name.

At any rate, I'll be back in this code in order to unify LoadIC and KeyedLoadIC
register specs. I'll pay the price for any failure to fully push through a
useful abstraction! And in doing that work I'll push the abstraction deeper
wherever possible.

+  ASSERT(ecx.is(NameRegister()));
+
   Label miss, slow;
 
   GenerateNameDictionaryReceiverCheck(masm, edx, eax, ebx, &miss);
 
   // eax: elements
   // Search the dictionary placing the result in eax.
   GenerateDictionaryLoad(masm, &slow, eax, ecx, edi, ebx, eax);
   __ ret(0);
 
   // Dictionary load failed, go slow (but don't miss).
   __ bind(&slow);
   GenerateRuntimeGetProperty(masm);
 
   // Cache miss: Jump to runtime.
   __ bind(&miss);
   GenerateMiss(masm);
 }
 
 
+// A register that isn't one of the parameters to the load ic.
+static const Register LoadIC_TempRegister() { return ebx; }

[Jakob Kummerow, 2014/06/25 10:59:12]

As discussed, I think medium-term we want to define this globally like
ReceiverRegister() and friends. But for now this is OK.

[mvstanton, 2014/06/25 15:10:27]

Okay, thanks. I will keep it this way for now. I'm reluctant to elevate temp
registers as a concept for the platform independent file to be concerned with.
Lets keep playing the refactoring out and the right home for these will become
clearer.

+
+
 void LoadIC::GenerateMiss(MacroAssembler* masm) {
-  // ----------- S t a t e -------------
-  //  -- ecx    : name
-  //  -- edx    : receiver
-  //  -- esp[0] : return address
-  // -----------------------------------
-
+  // ----------- S t a t e ------------------
+  //  -- ReceiverRegister()  : receiver

[Jakob Kummerow, 2014/06/25 10:59:12]

Arguably this entire comment block could now be reduced to:
// Return address is on the stack (esp[0]).
But I'm fine with keeping it.

[mvstanton, 2014/06/25 15:10:27]

It's a super-redundant comment, so I'll take it out. :D

+  //  -- NameRegister()      : name
+  //  -- esp[0]              : return address
+  // ----------------------------------------
   __ IncrementCounter(masm->isolate()->counters()->load_miss(), 1);
 
-  __ pop(ebx);
-  __ push(edx);  // receiver
-  __ push(ecx);  // name
-  __ push(ebx);  // return address
+  __ pop(LoadIC_TempRegister());
+  __ push(ReceiverRegister());  // receiver
+  __ push(NameRegister());  // name
+  __ push(LoadIC_TempRegister());  // return address
 
   // Perform tail call to the entry.
   ExternalReference ref =
       ExternalReference(IC_Utility(kLoadIC_Miss), masm->isolate());
   __ TailCallExternalReference(ref, 2, 1);
 }
 
 
 void LoadIC::GenerateRuntimeGetProperty(MacroAssembler* masm) {
-  // ----------- S t a t e -------------
-  //  -- ecx    : key
-  //  -- edx    : receiver
-  //  -- esp[0] : return address
-  // -----------------------------------
-
-  __ pop(ebx);
-  __ push(edx);  // receiver
-  __ push(ecx);  // name
-  __ push(ebx);  // return address
+  // ----------- S t a t e ------------------
+  //  -- ReceiverRegister()  : receiver
+  //  -- NameRegister()      : name
+  //  -- esp[0]              : return address
+  // ----------------------------------------
+  __ pop(LoadIC_TempRegister());
+  __ push(ReceiverRegister());   // receiver
+  __ push(NameRegister());       // name
+  __ push(LoadIC_TempRegister());  // return address
 
   // Perform tail call to the entry.
   __ TailCallRuntime(Runtime::kGetProperty, 2, 1);
 }
 
 
+// A register that isn't one of the parameters to the load ic.
+static const Register KeyedLoadIC_TempRegister() {
+  return LoadIC_TempRegister();
+}
+
+
 void KeyedLoadIC::GenerateMiss(MacroAssembler* masm) {
-  // ----------- S t a t e -------------
-  //  -- ecx    : key
-  //  -- edx    : receiver
-  //  -- esp[0] : return address
-  // -----------------------------------
-
+  // ----------- S t a t e ------------------
+  //  -- ReceiverRegister()  : receiver
+  //  -- NameRegister()      : name
+  //  -- esp[0]              : return address
+  // ----------------------------------------
   __ IncrementCounter(masm->isolate()->counters()->keyed_load_miss(), 1);
 
-  __ pop(ebx);
-  __ push(edx);  // receiver
-  __ push(ecx);  // name
-  __ push(ebx);  // return address
+  __ pop(KeyedLoadIC_TempRegister());
+  __ push(ReceiverRegister());  // receiver
+  __ push(NameRegister());  // name
+  __ push(KeyedLoadIC_TempRegister());  // return address
 
   // Perform tail call to the entry.
   ExternalReference ref =
       ExternalReference(IC_Utility(kKeyedLoadIC_Miss), masm->isolate());
   __ TailCallExternalReference(ref, 2, 1);
 }
 
 
 // IC register specifications
 const Register LoadIC::ReceiverRegister() { return edx; }
 const Register LoadIC::NameRegister() { return ecx; }
 
 
 const Register KeyedLoadIC::ReceiverRegister() {
   return LoadIC::ReceiverRegister();
 }
 
 
 const Register KeyedLoadIC::NameRegister() { return LoadIC::NameRegister(); }
 
 
 void KeyedLoadIC::GenerateRuntimeGetProperty(MacroAssembler* masm) {
-  // ----------- S t a t e -------------
-  //  -- ecx    : key
-  //  -- edx    : receiver
-  //  -- esp[0] : return address
-  // -----------------------------------
-
-  __ pop(ebx);
-  __ push(edx);  // receiver
-  __ push(ecx);  // name
-  __ push(ebx);  // return address
+  // ----------- S t a t e ------------------
+  //  -- ReceiverRegister()  : receiver
+  //  -- NameRegister()      : name
+  //  -- esp[0]              : return address
+  // ----------------------------------------
+  __ pop(KeyedLoadIC_TempRegister());
+  __ push(ReceiverRegister());  // receiver
+  __ push(NameRegister());  // name
+  __ push(KeyedLoadIC_TempRegister());  // return address
 
   // Perform tail call to the entry.
   __ TailCallRuntime(Runtime::kKeyedGetProperty, 2, 1);
 }
 
 
 void StoreIC::GenerateMegamorphic(MacroAssembler* masm) {
   // ----------- S t a t e -------------
   //  -- eax    : value
   //  -- ecx    : name
@@ skipping 232 matching lines @@
   Condition cc = (check == ENABLE_INLINED_SMI_CHECK)
       ? (*jmp_address == Assembler::kJncShortOpcode ? not_zero : zero)
       : (*jmp_address == Assembler::kJnzShortOpcode ? not_carry : carry);
   *jmp_address = static_cast<byte>(Assembler::kJccShortPrefix | cc);
 }
 
 
 } }  // namespace v8::internal
 
 #endif  // V8_TARGET_ARCH_IA32