As discussed, I would use an oddball rather than a fixed array.  You also
have to be careful about the hole value used to mark not-yet-allocated lazy
arguments in the classic backend.

On Thu, Jan 6, 2011 at 1:07 PM, <[email protected]> wrote:

> Reviewers: Kevin Millikin,
>
> Description:
> Use a separate marker value to allocate the arguments object on
> deoptimzation.
>
> Before we used the hole value for this purpose, but this does not work once
> we
> start using the hole value for other purposes in the optimizing compiler.
>
>
> Please review this at http://codereview.chromium.org/6116001/
>
> SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
>
> Affected files:
>  M     src/accessors.cc
>  M     src/deoptimizer.cc
>  M     src/heap.h
>  M     src/heap.cc
>  M     src/runtime.cc
>
>
> Index: src/accessors.cc
> ===================================================================
> --- src/accessors.cc    (revision 6194)
> +++ src/accessors.cc    (working copy)
> @@ -775,7 +775,7 @@
>         if (index >= 0) {
>           Handle<Object> arguments =
>               Handle<Object>(frame->GetExpression(index));
> -          if (!arguments->IsTheHole()) return *arguments;
> +          if (*arguments != Heap::arguments_marker()) return *arguments;
>         }
>
>         // If there isn't an arguments variable in the stack, we need to
> Index: src/deoptimizer.cc
> ===================================================================
> --- src/deoptimizer.cc  (revision 6194)
> +++ src/deoptimizer.cc  (working copy)
> @@ -618,17 +618,17 @@
>     }
>
>     case Translation::ARGUMENTS_OBJECT: {
> -      // Use the hole value as a sentinel and fill in the arguments object
> -      // after the deoptimized frame is built.
> +      // Use the arguments marker value as a sentinel and fill in the
> arguments
> +      // object after the deoptimized frame is built.
>       ASSERT(frame_index == 0);  // Only supported for first frame.
>       if (FLAG_trace_deopt) {
>         PrintF("    0x%08" V8PRIxPTR ": [top + %d] <- ",
>                output_[frame_index]->GetTop() + output_offset,
>                output_offset);
> -        Heap::the_hole_value()->ShortPrint();
> +        Heap::arguments_marker()->ShortPrint();
>         PrintF(" ; arguments object\n");
>       }
> -      intptr_t value = reinterpret_cast<intptr_t>(Heap::the_hole_value());
> +      intptr_t value =
> reinterpret_cast<intptr_t>(Heap::arguments_marker());
>       output_[frame_index]->SetFrameSlot(output_offset, value);
>       return;
>     }
> Index: src/heap.cc
> ===================================================================
> --- src/heap.cc (revision 6194)
> +++ src/heap.cc (working copy)
> @@ -2011,6 +2011,11 @@
>   }
>   set_the_hole_value(obj);
>
> +  { MaybeObject* maybe_obj = AllocateFixedArray(1);
> +    if (!maybe_obj->ToObject(&obj)) return false;
> +  }
> +  set_arguments_marker(obj);
> +
>   { MaybeObject* maybe_obj =
>         CreateOddball("no_interceptor_result_sentinel", Smi::FromInt(-2));
>     if (!maybe_obj->ToObject(&obj)) return false;
> Index: src/heap.h
> ===================================================================
> --- src/heap.h  (revision 6194)
> +++ src/heap.h  (working copy)
> @@ -116,6 +116,7 @@
>   V(Script, empty_script, EmptyScript)
>     \
>   V(Smi, real_stack_limit, RealStackLimit)
>     \
>   V(StringDictionary, intrinsic_function_names, IntrinsicFunctionNames)
>    \
> +  V(Object, arguments_marker, ArgumentsMarker)
>
>  #if V8_TARGET_ARCH_ARM && !V8_INTERPRETED_REGEXP
>  #define STRONG_ROOT_LIST(V)
>      \
> Index: src/runtime.cc
> ===================================================================
> --- src/runtime.cc      (revision 6194)
> +++ src/runtime.cc      (working copy)
> @@ -6742,7 +6742,7 @@
>   Handle<JSFunction> function(JSFunction::cast(frame->function()));
>   Handle<Object> arguments;
>   for (int i = frame->ComputeExpressionsCount() - 1; i >= 0; --i) {
> -    if (frame->GetExpression(i) == Heap::the_hole_value()) {
> +    if (frame->GetExpression(i) == Heap::arguments_marker()) {
>       if (arguments.is_null()) {
>         // FunctionGetArguments can't throw an exception, so cast away the
>         // doubt with an assert.
>
>
>

-- 
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to