Add more details the --trace-deopt output

Add more details the --trace-deopt output

R=ager@chromium.org
Committed: http://code.google.com/p/v8/source/detail?r=7387

[Mads Ager (chromium), 2011-03-28 07:43:11]

LGTM

http://codereview.chromium.org/6735007/diff/2002/src/ia32/deoptimizer-ia32.cc
File src/ia32/deoptimizer-ia32.cc (right):

http://codereview.chromium.org/6735007/diff/2002/src/ia32/deoptimizer-ia32.cc...
src/ia32/deoptimizer-ia32.cc:596:
PrintF("Deoptimizer::EntryGenerator::Generate()\n");
Remove?

[Kevin Millikin (Chromium), 2011-03-28 09:07:01]

Drive by suggestion.

http://codereview.chromium.org/6735007/diff/2002/src/arm/deoptimizer-arm.cc
File src/arm/deoptimizer-arm.cc (right):

http://codereview.chromium.org/6735007/diff/2002/src/arm/deoptimizer-arm.cc#n...
src/arm/deoptimizer-arm.cc:295: switch (i) {
Maybe it would be nicer to use the constants in frames-arm.h.  This bit of the
frame is uninterpreted by OSR, so it doesn't actually matter what "i"
corresponds to.  You could do something like:

for (int i = 1; ok && i >= -2; --i) {
  ...
  if (FLAG_trace_osr) {
    const char* name = "UNKNOWN";
    switch (i * kPointerSize) {
      case StandardFrameConstants::kCallerPCOffset:
        name = "caller's pc";
        break;
      case StandardFrameConstants::kCallerFPOffset:
        name = "caller's fp";
        break;
      case StandardFrameConstants::kContextOffset:
        name = "context";
        break;
      case JavaScriptFrameConstants::kFunctionOffset:
        name = "function";
        break;
    }

[Søren Thygesen Gjesse, 2011-03-28 12:27:29]

http://codereview.chromium.org/6735007/diff/2002/src/arm/deoptimizer-arm.cc
File src/arm/deoptimizer-arm.cc (right):

http://codereview.chromium.org/6735007/diff/2002/src/arm/deoptimizer-arm.cc#n...
src/arm/deoptimizer-arm.cc:295: switch (i) {
On 2011/03/28 09:07:01, Kevin Millikin wrote:
> Maybe it would be nicer to use the constants in frames-arm.h.  This bit of the
> frame is uninterpreted by OSR, so it doesn't actually matter what "i"
> corresponds to.  You could do something like:
> 
> for (int i = 1; ok && i >= -2; --i) {
>   ...
>   if (FLAG_trace_osr) {
>     const char* name = "UNKNOWN";
>     switch (i * kPointerSize) {
>       case StandardFrameConstants::kCallerPCOffset:
>         name = "caller's pc";
>         break;
>       case StandardFrameConstants::kCallerFPOffset:
>         name = "caller's fp";
>         break;
>       case StandardFrameConstants::kContextOffset:
>         name = "context";
>         break;
>       case JavaScriptFrameConstants::kFunctionOffset:
>         name = "function";
>         break;
>     }

Good point, used

  for (int i =  StandardFrameConstants::kCallerPCOffset;
       ok && i >=  StandardFrameConstants::kMarkerOffset;
       i -= kPointerSize) {

for the looping.

http://codereview.chromium.org/6735007/diff/2002/src/ia32/deoptimizer-ia32.cc
File src/ia32/deoptimizer-ia32.cc (right):

http://codereview.chromium.org/6735007/diff/2002/src/ia32/deoptimizer-ia32.cc...
src/ia32/deoptimizer-ia32.cc:596:
PrintF("Deoptimizer::EntryGenerator::Generate()\n");
On 2011/03/28 07:43:11, Mads Ager wrote:
> Remove?

Done.

[Kevin Millikin (Chromium), 2011-03-28 13:11:40]

http://codereview.chromium.org/6735007/diff/2002/src/arm/deoptimizer-arm.cc
File src/arm/deoptimizer-arm.cc (right):

http://codereview.chromium.org/6735007/diff/2002/src/arm/deoptimizer-arm.cc#n...
src/arm/deoptimizer-arm.cc:295: switch (i) {
On 2011/03/28 12:27:29, Søren Gjesse wrote:
> On 2011/03/28 09:07:01, Kevin Millikin wrote:
> > Maybe it would be nicer to use the constants in frames-arm.h.  This bit of
the
> > frame is uninterpreted by OSR, so it doesn't actually matter what "i"
> > corresponds to.  You could do something like:
> > 
> > for (int i = 1; ok && i >= -2; --i) {
> >   ...
> >   if (FLAG_trace_osr) {
> >     const char* name = "UNKNOWN";
> >     switch (i * kPointerSize) {
> >       case StandardFrameConstants::kCallerPCOffset:
> >         name = "caller's pc";
> >         break;
> >       case StandardFrameConstants::kCallerFPOffset:
> >         name = "caller's fp";
> >         break;
> >       case StandardFrameConstants::kContextOffset:
> >         name = "context";
> >         break;
> >       case JavaScriptFrameConstants::kFunctionOffset:
> >         name = "function";
> >         break;
> >     }
> 
> Good point, used
> 
>   for (int i =  StandardFrameConstants::kCallerPCOffset;
>        ok && i >=  StandardFrameConstants::kMarkerOffset;
>        i -= kPointerSize) {
> 
> for the looping.

Sounds good.