LGTM.

On Fri, Oct 24, 2008 at 11:32 AM,  <[EMAIL PROTECTED]> wrote:
> Kasper & Søren,
>
> I'd like you to do a code review.  To review this change, run
>
>  gvn review --project https://v8.googlecode.com/svn [EMAIL PROTECTED]/[EMAIL 
> PROTECTED]
>
> Alternatively, to review the latest snapshot of this change
> branch, run
>
>  gvn --project https://v8.googlecode.com/svn review [EMAIL 
> PROTECTED]/runtime_calls
>
> to review the following change:
>
> [EMAIL PROTECTED]/[EMAIL PROTECTED] | [EMAIL PROTECTED] | 2008-10-24 10:28:30 
> +-100 (Fri, 24 Oct 2008)
>
> Description:
>
> Change a few runtime functions that took and returned a dummy argument
> to instead take no arguments and return the undefined value.
>
>
>
> Affected Paths:
>   M //branches/bleeding_edge/src/codegen-arm.cc
>   M //branches/bleeding_edge/src/codegen-ia32.cc
>   M //branches/bleeding_edge/src/runtime.cc
>   M //branches/bleeding_edge/src/runtime.h
>
>
> This is a semiautomated message from "gvn mail".  See
> <http://code.google.com/p/gvn/> to learn more.
>
> Index: src/codegen-arm.cc
> ===================================================================
> --- src/codegen-arm.cc  (^/branches/bleeding_edge/src/[EMAIL PROTECTED])
> +++ src/codegen-arm.cc  (^/changes/[EMAIL 
> PROTECTED]/runtime_calls/bleeding_edge/src/[EMAIL PROTECTED])
> @@ -226,9 +226,7 @@ void CodeGenerator::GenCode(FunctionLiteral* fun)
>     if (FLAG_trace) {
>       // Push a valid value as the parameter. The runtime call only uses
>       // it as the return value to indicate non-failure.
> -      __ mov(r0, Operand(Smi::FromInt(0)));
> -      __ push(r0);
> -      __ CallRuntime(Runtime::kTraceEnter, 1);
> +      __ CallRuntime(Runtime::kTraceEnter, 0);
>     }
>     CheckStack();
>
> @@ -242,11 +240,7 @@ void CodeGenerator::GenCode(FunctionLiteral* fun)
>       bool should_trace =
>           is_builtin ? FLAG_trace_builtin_calls : FLAG_trace_calls;
>       if (should_trace) {
> -        // Push a valid value as the parameter. The runtime call only uses
> -        // it as the return value to indicate non-failure.
> -        __ mov(r0, Operand(Smi::FromInt(0)));
> -        __ push(r0);
> -        __ CallRuntime(Runtime::kDebugTrace, 1);
> +        __ CallRuntime(Runtime::kDebugTrace, 0);
>       }
>  #endif
>       VisitStatements(body);
> @@ -1846,8 +1840,8 @@ void CodeGenerator::VisitTryFinally(TryFinally* no
>  void CodeGenerator::VisitDebuggerStatement(DebuggerStatement* node) {
>   Comment cmnt(masm_, "[ DebuggerStatament");
>   if (FLAG_debug_info) RecordStatementPosition(node);
> -  __ CallRuntime(Runtime::kDebugBreak, 1);
> -  __ push(r0);
> +  __ CallRuntime(Runtime::kDebugBreak, 0);
> +  // Ignore the return value.
>  }
>
>
> Index: src/codegen-ia32.cc
> ===================================================================
> --- src/codegen-ia32.cc (^/branches/bleeding_edge/src/[EMAIL PROTECTED])
> +++ src/codegen-ia32.cc (^/changes/[EMAIL 
> PROTECTED]/runtime_calls/bleeding_edge/src/[EMAIL PROTECTED])
> @@ -356,8 +356,8 @@ void CodeGenerator::GenCode(FunctionLiteral* fun)
>     }
>
>     if (FLAG_trace) {
> -      __ CallRuntime(Runtime::kTraceEnter, 1);
> -      frame_->Push(eax);
> +      __ CallRuntime(Runtime::kTraceEnter, 0);
> +      // Ignore the return value.
>     }
>     CheckStack();
>
> @@ -371,8 +371,8 @@ void CodeGenerator::GenCode(FunctionLiteral* fun)
>       bool should_trace =
>           is_builtin ? FLAG_trace_builtin_calls : FLAG_trace_calls;
>       if (should_trace) {
> -        __ CallRuntime(Runtime::kDebugTrace, 1);
> -        frame_->Push(eax);
> +        __ CallRuntime(Runtime::kDebugTrace, 0);
> +        // Ignore the return value.
>       }
>  #endif
>       VisitStatements(body);
> @@ -2218,8 +2218,8 @@ void CodeGenerator::VisitTryFinally(TryFinally* no
>  void CodeGenerator::VisitDebuggerStatement(DebuggerStatement* node) {
>   Comment cmnt(masm_, "[ DebuggerStatement");
>   RecordStatementPosition(node);
> -  __ CallRuntime(Runtime::kDebugBreak, 1);
> -  frame_->Push(eax);
> +  __ CallRuntime(Runtime::kDebugBreak, 0);
> +  // Ignore the return value.
>  }
>
>
> Index: src/runtime.cc
> ===================================================================
> --- src/runtime.cc      (^/branches/bleeding_edge/src/[EMAIL PROTECTED])
> +++ src/runtime.cc      (^/changes/[EMAIL 
> PROTECTED]/runtime_calls/bleeding_edge/src/[EMAIL PROTECTED])
> @@ -3581,10 +3581,10 @@ static Object* RuntimePreempt(Arguments args) {
>  }
>
>
> -static Object* Runtime_DebugBreak(Arguments args) {
> +static Object* DebugBreakHelper() {
>   // Just continue if breaks are disabled.
>   if (Debug::disable_break()) {
> -    return args[0];
> +    return Heap::undefined_value();
>   }
>
>   // Don't break in system functions. If the current function is
> @@ -3596,7 +3596,7 @@ static Object* RuntimePreempt(Arguments args) {
>   if (fun->IsJSFunction()) {
>     GlobalObject* global = JSFunction::cast(fun)->context()->global();
>     if (global->IsJSBuiltinsObject() || Debug::IsDebugGlobal(global)) {
> -      return args[0];
> +      return Heap::undefined_value();
>     }
>   }
>
> @@ -3607,17 +3607,23 @@ static Object* RuntimePreempt(Arguments args) {
>   // Enter the debugger. Just continue if we fail to enter the debugger.
>   EnterDebugger debugger;
>   if (debugger.FailedToEnter()) {
> -    return args[0];
> +    return Heap::undefined_value();
>   }
>
>   // Notify the debug event listeners.
>   Debugger::OnDebugBreak(Factory::undefined_value());
>
>   // Return to continue execution.
> -  return args[0];
> +  return Heap::undefined_value();
>  }
>
>
> +static Object* Runtime_DebugBreak(Arguments args) {
> +  ASSERT(args.length() == 0);
> +  return DebugBreakHelper();
> +}
> +
> +
>  static Object* Runtime_StackGuard(Arguments args) {
>   ASSERT(args.length() == 1);
>
> @@ -3626,7 +3632,7 @@ static Object* Runtime_StackGuard(Arguments args)
>
>   // If not real stack overflow the stack guard was used to interrupt
>   // execution for another purpose.
> -  if (StackGuard::IsDebugBreak()) Runtime_DebugBreak(args);
> +  if (StackGuard::IsDebugBreak()) DebugBreakHelper();
>   if (StackGuard::IsPreempted()) RuntimePreempt(args);
>   if (StackGuard::IsInterrupted()) {
>     // interrupt
> @@ -3724,9 +3730,10 @@ static void PrintTransition(Object* result) {
>
>
>  static Object* Runtime_TraceEnter(Arguments args) {
> +  ASSERT(args.length() == 0);
>   NoHandleAllocation ha;
>   PrintTransition(NULL);
> -  return args[0];  // return TOS
> +  return Heap::undefined_value();
>  }
>
>
> @@ -3765,10 +3772,10 @@ static Object* Runtime_DebugPrint(Arguments args)
>
>
>  static Object* Runtime_DebugTrace(Arguments args) {
> -  ASSERT(args.length() == 1);
> +  ASSERT(args.length() == 0);
>   NoHandleAllocation ha;
>   Top::PrintStack();
> -  return args[0];  // return TOS
> +  return Heap::undefined_value();
>  }
>
>
> Index: src/runtime.h
> ===================================================================
> --- src/runtime.h       (^/branches/bleeding_edge/src/[EMAIL PROTECTED])
> +++ src/runtime.h       (^/changes/[EMAIL 
> PROTECTED]/runtime_calls/bleeding_edge/src/[EMAIL PROTECTED])
> @@ -277,10 +277,10 @@ namespace v8 { namespace internal {
>   \
>   /* Debugging */ \
>   F(DebugPrint, 1) \
> -  F(DebugTrace, 1) \
> -  F(TraceEnter, 1) \
> +  F(DebugTrace, 0) \
> +  F(TraceEnter, 0) \
>   F(TraceExit, 1) \
> -  F(DebugBreak, 1) \
> +  F(DebugBreak, 0) \
>   F(FunctionGetAssemblerCode, 1) \
>   F(Abort, 2) \
>   \
>
>

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

Reply via email to