Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(26)

Issue 7327038: Add source position recording for variable loads (Closed)

Created:
9 years, 5 months ago by Søren Thygesen Gjesse
Modified:
9 years, 5 months ago
CC:
v8-dev
Visibility:
Public.

Description

Add source position recording for variable loads This provides more precise source to generated code mapping as variable loads can be handled using IC calls. R=kmillikin@chromium.org BUG=v8:1527 TEST=test/message/regress/regress-1527 Committed: http://code.google.com/p/v8/source/detail?r=8610

Patch Set 1 #

Total comments: 6

Patch Set 2 : Addressed review comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+110 lines, -28 lines) Patch
M src/arm/full-codegen-arm.cc View 5 chunks +9 lines, -5 lines 0 comments Download
M src/full-codegen.h View 1 chunk +1 line, -1 line 0 comments Download
M src/ia32/full-codegen-ia32.cc View 6 chunks +10 lines, -6 lines 0 comments Download
M src/mips/full-codegen-mips.cc View 5 chunks +9 lines, -5 lines 0 comments Download
M src/x64/full-codegen-x64.cc View 5 chunks +9 lines, -5 lines 0 comments Download
M test/cctest/test-api.cc View 1 2 chunks +6 lines, -4 lines 0 comments Download
A test/message/regress/regress-1527.js View 1 chunk +33 lines, -0 lines 0 comments Download
A test/message/regress/regress-1527.out View 1 1 chunk +32 lines, -0 lines 0 comments Download
M test/mjsunit/debug-backtrace.js View 2 chunks +1 line, -2 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Søren Thygesen Gjesse
9 years, 5 months ago (2011-07-11 15:07:20 UTC) #1
Kevin Millikin (Chromium)
LGTM with the small comments below. http://codereview.chromium.org/7327038/diff/1/test/cctest/test-api.cc File test/cctest/test-api.cc (right): http://codereview.chromium.org/7327038/diff/1/test/cctest/test-api.cc#newcode12639 test/cctest/test-api.cc:12639: // The last ...
9 years, 5 months ago (2011-07-11 15:11:08 UTC) #2
Søren Thygesen Gjesse
9 years, 5 months ago (2011-07-11 15:19:16 UTC) #3
http://codereview.chromium.org/7327038/diff/1/test/cctest/test-api.cc
File test/cctest/test-api.cc (right):

http://codereview.chromium.org/7327038/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:12639: // The last frame is an anonymous function which
have the initial eval call.
On 2011/07/11 15:11:08, Kevin Millikin wrote:
> have => has

Done.

http://codereview.chromium.org/7327038/diff/1/test/cctest/test-api.cc#newcode...
test/cctest/test-api.cc:12658: // This is the source string inside the eval
which have the call to baz.
On 2011/07/11 15:11:08, Kevin Millikin wrote:
> have => has, also just below.

Done.

http://codereview.chromium.org/7327038/diff/1/test/message/regress/regress-15...
File test/message/regress/regress-1527.out (right):

http://codereview.chromium.org/7327038/diff/1/test/message/regress/regress-15...
test/message/regress/regress-1527.out:1: # Copyright 2008 the V8 project
authors. All rights reserved.
On 2011/07/11 15:11:08, Kevin Millikin wrote:
> Cut and paste a different copyright header :)

Done.

Powered by Google App Engine
This is Rietveld 408576698