Fast-codegen: Added support for arguments in functions.

src/arm/fast-codegen-arm.cc

 // Copyright 2009 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 53 matching lines @@
     int locals_count = fun->scope()->num_stack_slots();
     if (locals_count > 0) {
       __ LoadRoot(ip, Heap::kUndefinedValueRootIndex);
     }
     __ LoadRoot(r2, Heap::kStackLimitRootIndex);
     for (int i = 0; i < locals_count; i++) {
       __ push(ip);
     }
   }
 
+  bool function_in_register = true;
+
+  Variable* arguments = fun->scope()->arguments()->AsVariable();
+  if (arguments != NULL) {
+    // Function uses arguments object.
+    Comment cmnt(masm_, "[ Allocate arguments object");
+    __ mov(r3, r1);
+    // Receiver is located above the frame pointer, return address and

[Kevin Millikin (Chromium), 2009/11/12 14:23:13]

Like Bill, I think you should avoid "above" and "below" in stack comments, or
else be consistent with the rest of the codegen comments (usually, we push
something "on top" of something else and thus it is "above", regardless of
address).

Receiver is parameter number -1.

[Lasse Reichstein, 2009/11/13 08:54:37]

I committed this comment: "Receiver is just before the parameters on the
caller's stack."
(and the number it comments is "StandardFrameConstants::kCallerSPOffset +
fun->num_parameters() * kPointerSize".

How does this read?

+    // parameters.
+    __ add(r2, fp, Operand((fun->num_parameters() + 2) * kPointerSize));
+    __ mov(r1, Operand(Smi::FromInt(fun->num_parameters())));
+    __ stm(db_w, sp, r1.bit() | r2.bit() | r3.bit());

[Kevin Millikin (Chromium), 2009/11/12 14:23:13]

When using stm to push arguments, list the registers in argument order like you
were making a function call; ie, r3, r2, r1 here.

[Lasse Reichstein, 2009/11/13 08:54:37]

Will fix.

+
+    // Arguments to ArgumentsAccessStub:
+    //   function, receiver address, parameter count.
+    // The stub will rewrite receiever and parameter count if the previous
+    // stack frame was an arguments adapter frame.
+    ArgumentsAccessStub stub(ArgumentsAccessStub::NEW_OBJECT);
+    __ CallStub(&stub);
+    Slot* arguments_slot = arguments->slot();
+    __ str(r0, MemOperand(fp, SlotOffset(arguments_slot)));
+    Slot* dot_arguments_slot =
+        fun->scope()->arguments_shadow()->AsVariable()->slot();
+    __ str(r0, MemOperand(fp, SlotOffset(dot_arguments_slot)));
+    function_in_register = false;

[Kevin Millikin (Chromium), 2009/11/12 14:23:13]

Isn't function_in_register == (arguments != NULL)?  I find it more
straightforward to just use that fact down below.

[Lasse Reichstein, 2009/11/13 08:54:37]

It is. I preferred it this way because it avoided interleaving the two,
otherwise independent, blocks.

If anyone wants to add something between them later, they will need to know that
the meaning of "arguments != NULL" is that the function is no longer in register
r1, as well as why that is the case.

+  }
+
   // Possibly allocate a local context.
   if (fun->scope()->num_heap_slots() > 0) {
     Comment cmnt(masm_, "[ Allocate local context");
     // Argument to NewContext is the function, still in r1.

[Kevin Millikin (Chromium), 2009/11/12 14:23:13]

Comment is wrong.

[Lasse Reichstein, 2009/11/13 08:54:37]

Fixed.

+    if (!function_in_register) {
+      __ ldr(r1, MemOperand(fp, JavaScriptFrameConstants::kFunctionOffset));
+    }
     __ push(r1);
     __ CallRuntime(Runtime::kNewContext, 1);
     // Context is returned in both r0 and cp.  It replaces the context
     // passed to us.  It's saved in the stack and kept live in cp.
     __ str(cp, MemOperand(fp, StandardFrameConstants::kContextOffset));
 #ifdef DEBUG
     // Assert we do not have to copy any parameters into the context.
     for (int i = 0, len = fun->scope()->num_parameters(); i < len; i++) {
       Slot* slot = fun->scope()->parameter(i)->slot();
       ASSERT(slot != NULL && slot->type() != Slot::CONTEXT);
@@ skipping 337 matching lines @@
     ASSERT(expr->var()->is_global());
     Comment cmnt(masm_, "Global variable");
     // Use inline caching. Variable name is passed in r2 and the global
     // object on the stack.
     __ ldr(ip, CodeGenerator::GlobalObject());
     __ push(ip);
     __ mov(r2, Operand(expr->name()));
     Handle<Code> ic(Builtins::builtin(Builtins::LoadIC_Initialize));
     __ Call(ic, RelocInfo::CODE_TARGET_CONTEXT);
     DropAndMove(expr->context(), r0);
-  } else {
+  } else if (rewrite->AsSlot() != NULL) {
     Slot* slot = rewrite->AsSlot();
     ASSERT_NE(NULL, slot);
     switch (slot->type()) {
       case Slot::LOCAL:
       case Slot::PARAMETER: {
         Comment cmnt(masm_, "Stack slot");
         Move(expr->context(), rewrite->AsSlot());
         break;
       }
 
@@ skipping 21 matching lines @@
         }
         __ ldr(r0, CodeGenerator::ContextOperand(r0, slot->index()));
         Move(expr->context(), r0);
         break;
       }
 
       case Slot::LOOKUP:
         UNREACHABLE();
         break;
     }
+  } else {
+    // The parameter variable has been rewritten into an explict access to
+    // the arguments object.

[Kevin Millikin (Chromium), 2009/11/12 14:23:13]

Here we don't know or care that it's the arguments object, do we?

I could easily see us changing the way we do variable resolution and rewrites,
and then this remote comment would make no sense.

[Lasse Reichstein, 2009/11/13 08:54:37]

True. It's currently the only reason, but we could prepare for other reasons for
this rewrite (or other expression rewrites, removing the assertion that it's a
property).
I'll reword the comment.

+    Property* property = rewrite->AsProperty();
+    ASSERT_NOT_NULL(property);
+    ASSERT_EQ(expr->context(), property->context());
+    Visit(property);
   }
 }
 
 
 void FastCodeGenerator::VisitRegExpLiteral(RegExpLiteral* expr) {
   Comment cmnt(masm_, "[ RegExpLiteral");
   Label done;
   // Registers will be used as follows:
   // r4 = JS function, literals array
   // r3 = literal index
@@ skipping 1038 matching lines @@
   true_label_ = saved_true;
   false_label_ = saved_false;
   // Convert current context to test context: End post-test code.
 }
 
 
 #undef __
 
 
 } }  // namespace v8::internal