Refactored the recording of source position in the generated code. The code g...

Refactored the recording of source position in the generated code. The code generator now has two methods

  void CodeForStatement(Node* node)
  void CodeForSourcePosition(int pos)

The first is used to indicate that code is about to be generated for the given statement and the second is used to indicate that code is about to be generated for the given source position.

Added position information for some statements which was missing whem.

Updated the code generator for ARM to emit source position the same way as for IA-32.

Added an assert to ensure that deferred code stubs will always have a source source position as if it has not it will take whatever source position before which makes no sense.

The passing test on ARM has only been tested using the simulator.

Committed: http://code.google.com/p/v8/source/detail?r=985

[Søren Thygesen Gjesse, 2008-12-16 22:19:37]

This also works with your experimental patch.

[Mads Ager (chromium), 2008-12-17 07:45:35]

LGTM

http://codereview.chromium.org/14170/diff/1/9
File src/assembler-ia32.cc (right):

http://codereview.chromium.org/14170/diff/1/9#newcode1993
Line 1993: ASSERT(pos != RelocInfo::kNoPosition);
In the ARM code, RecordPosition and RecordStatementPosition called
WriteRecordedPositions.  Why is that not needed here (or is it not needed on
ARM)?

http://codereview.chromium.org/14170/diff/1/3
File src/assembler-ia32.h (right):

http://codereview.chromium.org/14170/diff/1/3#newcode710
Line 710: int current_statement_position() const  { return
current_statement_position_; }
This line looks a tiny bit too long?

[Søren Thygesen Gjesse, 2008-12-17 08:20:26]

http://codereview.chromium.org/14170/diff/1/9
File src/assembler-ia32.cc (right):

http://codereview.chromium.org/14170/diff/1/9#newcode1993
Line 1993: ASSERT(pos != RelocInfo::kNoPosition);
On 2008/12/17 07:45:35, Mads Ager wrote:
> In the ARM code, RecordPosition and RecordStatementPosition called
> WriteRecordedPositions.  Why is that not needed here (or is it not needed on
> ARM)?

The reason for this it that on Intel we postpone the actual writing to the
relocation information until the point where there can actually be a debug
break. Currently it means that the positions are written whenever either a call
instruction  is written or when the JavaScript return relocation information
record is written. The reason for postponing it on Intel was to make the
peephole optimizer more efficient as it could not eliminate instructions which
har relocation information attached to them.

http://codereview.chromium.org/14170/diff/1/3
File src/assembler-ia32.h (right):

http://codereview.chromium.org/14170/diff/1/3#newcode710
Line 710: int current_statement_position() const  { return
current_statement_position_; }
On 2008/12/17 07:45:35, Mads Ager wrote:
> This line looks a tiny bit too long?

Done.

[iposva, 2008-12-17 08:30:39]

How much work would be involved in having ARM and IA32 assemblers match when
relocation info is generated?

-Ivan

http://codereview.chromium.org/14170/diff/1/9
File src/assembler-ia32.cc (right):

http://codereview.chromium.org/14170/diff/1/9#newcode1993
Line 1993: ASSERT(pos != RelocInfo::kNoPosition);
ARM also has a peephole optimizer so it should mirror what we do for IA32. We
have just not investigated the code quality as much at this point.

 2008/12/17 08:20:26, Søren Gjesse wrote:
> On 2008/12/17 07:45:35, Mads Ager wrote:
> > In the ARM code, RecordPosition and RecordStatementPosition called
> > WriteRecordedPositions.  Why is that not needed here (or is it not needed on
> > ARM)?
> 
> The reason for this it that on Intel we postpone the actual writing to the
> relocation information until the point where there can actually be a debug
> break. Currently it means that the positions are written whenever either a
call
> instruction  is written or when the JavaScript return relocation information
> record is written. The reason for postponing it on Intel was to make the
> peephole optimizer more efficient as it could not eliminate instructions which
> har relocation information attached to them.