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

Issue 1157213004: Drop computed handler count and index from AST. (Closed)

Created:
5 years, 6 months ago by Michael Starzinger
Modified:
5 years, 6 months ago
Reviewers:
Igor Sheludko
CC:
v8-dev
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Drop computed handler count and index from AST. These values were computed by the parser and hence out of sync with any visitor over the AST. Our AST visitor aborts visitation of statement lists as soon as a jump statement has been reached. Now handler tables are guaranteed to be dense and fully populated. R=ishell@chromium.org TEST=mjsunit/regress/regress-crbug-493290 BUG=chromium:493290 LOG=N Committed: https://crrev.com/c14ba5ec48f0b6327eaadc63837f1549fbb60bd2 Cr-Commit-Position: refs/heads/master@{#28846}

Patch Set 1 #

Patch Set 2 : Tiny cleanup. #

Patch Set 3 : Architecture ports. #

Patch Set 4 : Tiny cleanup. #

Total comments: 2

Patch Set 5 : Addressed comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+123 lines, -167 lines) Patch
M src/arm/full-codegen-arm.cc View 1 2 4 chunks +4 lines, -7 lines 0 comments Download
M src/arm64/full-codegen-arm64.cc View 1 2 4 chunks +4 lines, -7 lines 0 comments Download
M src/ast.h View 12 chunks +20 lines, -53 lines 0 comments Download
M src/full-codegen.h View 1 2 3 4 4 chunks +12 lines, -2 lines 0 comments Download
M src/full-codegen.cc View 1 2 3 4 6 chunks +42 lines, -13 lines 0 comments Download
M src/ia32/full-codegen-ia32.cc View 4 chunks +4 lines, -7 lines 0 comments Download
M src/mips/full-codegen-mips.cc View 1 2 4 chunks +4 lines, -7 lines 0 comments Download
M src/mips64/full-codegen-mips64.cc View 1 2 4 chunks +4 lines, -7 lines 0 comments Download
M src/parser.cc View 8 chunks +12 lines, -21 lines 0 comments Download
M src/ppc/full-codegen-ppc.cc View 1 2 4 chunks +4 lines, -7 lines 0 comments Download
M src/preparser.h View 10 chunks +3 lines, -19 lines 0 comments Download
M src/x64/full-codegen-x64.cc View 1 2 4 chunks +4 lines, -7 lines 0 comments Download
M src/x87/full-codegen-x87.cc View 1 2 4 chunks +4 lines, -7 lines 0 comments Download
A + test/mjsunit/regress/regress-crbug-493290.js View 1 chunk +2 lines, -3 lines 0 comments Download

Messages

Total messages: 9 (2 generated)
Michael Starzinger
Note that architecture ports are still missing. But otherwise this is ready for a review.
5 years, 6 months ago (2015-06-08 13:41:44 UTC) #1
Michael Starzinger
Ports done.
5 years, 6 months ago (2015-06-08 13:58:40 UTC) #2
Igor Sheludko
Nice! lgtm https://codereview.chromium.org/1157213004/diff/60001/src/full-codegen.cc File src/full-codegen.cc (right): https://codereview.chromium.org/1157213004/diff/60001/src/full-codegen.cc#newcode1485 src/full-codegen.cc:1485: entry->pc_offset = handler->pos(); nit: WDYT about calling ...
5 years, 6 months ago (2015-06-08 14:30:13 UTC) #3
Michael Starzinger
https://codereview.chromium.org/1157213004/diff/60001/src/full-codegen.cc File src/full-codegen.cc (right): https://codereview.chromium.org/1157213004/diff/60001/src/full-codegen.cc#newcode1485 src/full-codegen.cc:1485: entry->pc_offset = handler->pos(); On 2015/06/08 14:30:13, Igor Sheludko wrote: ...
5 years, 6 months ago (2015-06-08 14:34:27 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1157213004/80001
5 years, 6 months ago (2015-06-08 16:18:22 UTC) #7
commit-bot: I haz the power
Committed patchset #5 (id:80001)
5 years, 6 months ago (2015-06-08 18:19:43 UTC) #8
commit-bot: I haz the power
5 years, 6 months ago (2015-06-08 18:19:51 UTC) #9
Message was sent while issue was closed.
Patchset 5 (id:??) landed as
https://crrev.com/c14ba5ec48f0b6327eaadc63837f1549fbb60bd2
Cr-Commit-Position: refs/heads/master@{#28846}

Powered by Google App Engine
This is Rietveld 408576698