Port count-based profiler to ARM

Port count-based profiler to ARM

Committed: https://code.google.com/p/v8/source/detail?r=11120

[Jakob Kummerow, 2012-03-22 10:53:23]

This CL contains the ARM ports of the following concepts:

- count-based interrupts instead of stack checks
- "fake back edges" at function exit instead of self-optimization headers
- counting the number of ICs in each function, and the fraction that has type
information (changes of the sort s/__ Call/CallIC/)

All these are already implemented for ia32, but since they developed over time,
there are no CLs that introduced exactly these features. If you prefer, I can
certainly split this CL in three.

As a bonus, it contains a small fix that's also applied to ia32: when the
debugger is active, drastically reduce the interrupt budget in order to check
for debug breaks quicker. This fixes a test timeout that would otherwise happen
on ARM (cctest/test-debug/DebugBreakLoop).

[Erik Corry, 2012-03-23 09:55:52]

LGTM

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

http://codereview.chromium.org/9837004/diff/1/src/arm/deoptimizer-arm.cc#newc...
src/arm/deoptimizer-arm.cc:112: static const int32_t kBranchBeforeInterrupt = 
0x5a000004;
There is a bunch of these in assembler-arm.h eg kMovLrPc.  I think these should
go there too.

http://codereview.chromium.org/9837004/diff/1/src/arm/deoptimizer-arm.cc#newc...
src/arm/deoptimizer-arm.cc:170: (al | B24 | B21 | 15*B16 | 15*B12 | 15*B8 | BLX
| ip.code()));
What is this instruction?

http://codereview.chromium.org/9837004/diff/1/src/arm/full-codegen-arm.cc
File src/arm/full-codegen-arm.cc (right):

http://codereview.chromium.org/9837004/diff/1/src/arm/full-codegen-arm.cc#new...
src/arm/full-codegen-arm.cc:113: // TODO(jkummerow): Obsolete as soon as x64 is
updated. Remove.
Does this really have to do with x64?

http://codereview.chromium.org/9837004/diff/1/src/arm/full-codegen-arm.cc#new...
src/arm/full-codegen-arm.cc:354: weight = Min(127, Max(1, distance / 142));
These constants need names.

http://codereview.chromium.org/9837004/diff/1/src/arm/full-codegen-arm.cc#new...
src/arm/full-codegen-arm.cc:405: weight = Min(127, Max(1, distance / 142));
Here they are again!  (Or are these new constants that happen to have the same
values :-)

http://codereview.chromium.org/9837004/diff/1/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):

http://codereview.chromium.org/9837004/diff/1/src/ia32/full-codegen-ia32.cc#n...
src/ia32/full-codegen-ia32.cc:104: // TODO(jkummerow): Obsolete as soon as x64
is updated. Remove.
hmmm

[Jakob Kummerow, 2012-03-23 12:07:32]

Thanks for the review. I've addressed all of your comments, except for one that
I did not agree with (see below).

OK?

https://chromiumcodereview.appspot.com/9837004/diff/1/src/arm/deoptimizer-arm.cc
File src/arm/deoptimizer-arm.cc (right):

https://chromiumcodereview.appspot.com/9837004/diff/1/src/arm/deoptimizer-arm...
src/arm/deoptimizer-arm.cc:112: static const int32_t kBranchBeforeInterrupt = 
0x5a000004;
On 2012/03/23 09:55:53, Erik Corry wrote:
> There is a bunch of these in assembler-arm.h eg kMovLrPc.  I think these
should
> go there too.

I disagree. They contain condition codes and jump offsets, and are therefore
strictly tied to this usage site. I don't think they can ever be shared, so IMHO
pulling them up to a more global place reduces readability/understandability of
this code here.

I could split the definitions:
/*assembler-arm.{h|cc}*/
const Instr kBcs = 0x2a000000;
const Instr kBpl = 0x5a000000;
/*here/*
static const int32_t kBranchBeforeStackCheck = kBce | 1;
static const int32_t kBranchBeforeInterrupt = kBpl | 4;

Would you like that better?

https://chromiumcodereview.appspot.com/9837004/diff/1/src/arm/deoptimizer-arm...
src/arm/deoptimizer-arm.cc:170: (al | B24 | B21 | 15*B16 | 15*B12 | 15*B8 | BLX
| ip.code()));
On 2012/03/23 09:55:53, Erik Corry wrote:
> What is this instruction?

"blx ip", copied from above in the interest of unification, see also the
comments in PatchStackCheckCodeAt listing the relevant machine code.
Since this instruction is not specific to this site, I've moved it to
assembler-arm.h.

https://chromiumcodereview.appspot.com/9837004/diff/1/src/arm/full-codegen-ar...
File src/arm/full-codegen-arm.cc (right):

https://chromiumcodereview.appspot.com/9837004/diff/1/src/arm/full-codegen-ar...
src/arm/full-codegen-arm.cc:113: // TODO(jkummerow): Obsolete as soon as x64 is
updated. Remove.
On 2012/03/23 09:55:53, Erik Corry wrote:
> Does this really have to do with x64?

Yes. I don't want to break compilation before these changes are ported to all
architectures. Just wait for my next CL :-)

https://chromiumcodereview.appspot.com/9837004/diff/1/src/arm/full-codegen-ar...
src/arm/full-codegen-arm.cc:354: weight = Min(127, Max(1, distance / 142));
On 2012/03/23 09:55:53, Erik Corry wrote:
> These constants need names.
> 

Done.

https://chromiumcodereview.appspot.com/9837004/diff/1/src/arm/full-codegen-ar...
src/arm/full-codegen-arm.cc:405: weight = Min(127, Max(1, distance / 142));
On 2012/03/23 09:55:53, Erik Corry wrote:
> Here they are again!  (Or are these new constants that happen to have the same
> values :-)

Done.

https://chromiumcodereview.appspot.com/9837004/diff/1/src/ia32/full-codegen-i...
File src/ia32/full-codegen-ia32.cc (right):

https://chromiumcodereview.appspot.com/9837004/diff/1/src/ia32/full-codegen-i...
src/ia32/full-codegen-ia32.cc:104: // TODO(jkummerow): Obsolete as soon as x64
is updated. Remove.
On 2012/03/23 09:55:53, Erik Corry wrote:
> hmmm

See my response in full-codegen-arm.cc.