Inline %_ArgumentsLength.

src/ia32/codegen-ia32.cc

 // Copyright 2010 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 6210 matching lines @@
   __ bind(&check_frame_marker);
   __ cmp(Operand(fp.reg(), StandardFrameConstants::kMarkerOffset),
          Immediate(Smi::FromInt(StackFrame::CONSTRUCT)));
   fp.Unuse();
   destination()->Split(equal);
 }
 
 
 void CodeGenerator::GenerateArgumentsLength(ZoneList<Expression*>* args) {
   ASSERT(args->length() == 0);
-  // ArgumentsAccessStub takes the parameter count as an input argument
-  // in register eax.  Create a constant result for it.
-  Result count(Handle<Smi>(Smi::FromInt(scope()->num_parameters())));
-  // Call the shared stub to get to the arguments.length.
-  ArgumentsAccessStub stub(ArgumentsAccessStub::READ_LENGTH);
-  Result result = frame_->CallStub(&stub, &count);
+
+  Result fp = allocator_->Allocate();
+  Result result = allocator_->Allocate();

[antonm, 2010/03/30 13:28:05]

shouldn't you Unuse fp and result at the end of the function?  it looks like a
common convention.

[antonm, 2010/03/30 13:28:05]

just curious: any chances allocating the single reg would be faster even given
that control flow would become more involved?

[Vitaly Repeshko, 2010/03/30 13:40:30]

~Result will do it. I think it's only necessary when having some complicated
control flow that actually checks there are no off-frame values.

[Vitaly Repeshko, 2010/03/30 13:40:30]

Hard to tell. I can experiment more after this change is submitted.

[fschneider, 2010/03/30 13:44:36]

Unuse() is called by the destructor of Result at the end. In this case since we
don't allocate other things before that there is no need to explicitly call
Unuse.

+  ASSERT(fp.is_valid() && result.is_valid());
+
+  Label exit;
+
+  // Get the number of formal parameters.
+  __ Set(result.reg(), Immediate(Smi::FromInt(scope()->num_parameters())));
+
+  // Check if the calling frame is an arguments adaptor frame.
+  __ mov(fp.reg(), Operand(ebp, StandardFrameConstants::kCallerFPOffset));
+  __ cmp(Operand(fp.reg(), StandardFrameConstants::kContextOffset),
+         Immediate(Smi::FromInt(StackFrame::ARGUMENTS_ADAPTOR)));
+  __ j(not_equal, &exit);
+
+  // Arguments adaptor case: Read the arguments length from the
+  // adaptor frame.
+  __ mov(result.reg(),
+         Operand(fp.reg(), ArgumentsAdaptorFrameConstants::kLengthOffset));
+
+  __ bind(&exit);
+  result.set_type_info(TypeInfo::Smi());
+  if (FLAG_debug_code) __ AbortIfNotSmi(result.reg());

[antonm, 2010/03/30 13:28:05]

two questions.

1) why not use cmov still?
2) do we know what happens more often?  there is a redundant load to
result.reg(), of course, that's immediate so it hardly matters, just curious.

[Vitaly Repeshko, 2010/03/30 13:40:30]

AFAIK, cmov should be used in totally unpredictable cases and it made perfect
sense for the shared stub. Now that it's inlined I think the branch predictor
should be able to do a better job.
I think we get the expected number of arguments (i.e. equal to the number of
formal parameters) more often.

   frame_->Push(&result);
 }
 
 
 void CodeGenerator::GenerateClassOf(ZoneList<Expression*>* args) {
   ASSERT(args->length() == 1);
   JumpTarget leave, null, function, non_function_constructor;
   Load(args->at(0));  // Load the object.
   Result obj = frame_->Pop();
   obj.ToRegister();
@@ skipping 4157 matching lines @@
       break;
     case Token::BIT_NOT:
       __ InvokeBuiltin(Builtins::BIT_NOT, JUMP_FUNCTION);
       break;
     default:
       UNREACHABLE();
   }
 }
 
 
-void ArgumentsAccessStub::GenerateReadLength(MacroAssembler* masm) {
-  // Check if the calling frame is an arguments adaptor frame.
-  __ mov(edx, Operand(ebp, StandardFrameConstants::kCallerFPOffset));
-  __ mov(ecx, Operand(edx, StandardFrameConstants::kContextOffset));
-  __ cmp(Operand(ecx), Immediate(Smi::FromInt(StackFrame::ARGUMENTS_ADAPTOR)));
-
-  // Arguments adaptor case: Read the arguments length from the
-  // adaptor frame and return it.
-  // Otherwise nothing to do: The number of formal parameters has already been
-  // passed in register eax by calling function. Just return it.
-  if (CpuFeatures::IsSupported(CMOV)) {
-    CpuFeatures::Scope use_cmov(CMOV);
-    __ cmov(equal, eax,
-            Operand(edx, ArgumentsAdaptorFrameConstants::kLengthOffset));
-  } else {
-    Label exit;
-    __ j(not_equal, &exit);
-    __ mov(eax, Operand(edx, ArgumentsAdaptorFrameConstants::kLengthOffset));
-    __ bind(&exit);
-  }
-  __ ret(0);
-}
-
-
 void ArgumentsAccessStub::GenerateReadElement(MacroAssembler* masm) {
   // The key is in edx and the parameter count is in eax.
 
   // The displacement is used for skipping the frame pointer on the
   // stack. It is the offset of the last parameter (if any) relative
   // to the frame pointer.
   static const int kDisplacement = 1 * kPointerSize;
 
   // Check that the key is a smi.
   Label slow;
@@ skipping 2153 matching lines @@
 
   // Call the runtime; it returns -1 (less), 0 (equal), or 1 (greater)
   // tagged as a small integer.
   __ bind(&runtime);
   __ TailCallRuntime(Runtime::kStringCompare, 2, 1);
 }
 
 #undef __
 
 } }  // namespace v8::internal