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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 765983002: Clean up node iteration (Closed)

Created:
6 years ago by danno
Modified:
6 years ago
CC:
v8-dev
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Project:
v8
Visibility:
Public.

Description

Clean up node iteration - Create a first-class Edge type. - Separate node and edge iterators - Make iterators only responsible for iteration - Make it possible to modify the use edge iterator while iterating. - Add the ability to update inputs to Edges directly.

Patch Set 1 #

Patch Set 2 : Merge with ToT #

Patch Set 3 : Fix comment #

Total comments: 1

Patch Set 4 : Review feedback #

Patch Set 5 : Merge with ToT #

Patch Set 6 : Fix comments #

Patch Set 7 : Tweaks #

Total comments: 2

Patch Set 8 : Merge with ToT #

Patch Set 9 : Merge with ToT #

Patch Set 10 : Fix windows build #

Unified diffs Side-by-side diffs Delta from patch set Stats (+301 lines, -192 lines) Patch
M src/compiler/change-lowering.cc View 1 2 3 2 chunks +6 lines, -8 lines 0 comments Download
M src/compiler/control-reducer.cc View 1 2 3 4 5 6 7 2 chunks +16 lines, -19 lines 0 comments Download
M src/compiler/generic-algorithm.h View 1 2 3 4 1 chunk +2 lines, -2 lines 0 comments Download
M src/compiler/generic-algorithm-inl.h View 1 2 3 4 1 chunk +5 lines, -5 lines 0 comments Download
M src/compiler/graph-visualizer.cc View 1 2 3 4 5 chunks +11 lines, -11 lines 0 comments Download
M src/compiler/instruction-selector.cc View 1 2 3 4 5 6 7 2 chunks +3 lines, -5 lines 0 comments Download
M src/compiler/js-inlining.cc View 1 2 3 4 5 4 chunks +16 lines, -22 lines 0 comments Download
M src/compiler/node.h View 1 2 3 4 5 6 7 8 9 11 chunks +190 lines, -56 lines 0 comments Download
M src/compiler/node-properties.h View 2 chunks +5 lines, -5 lines 0 comments Download
M src/compiler/node-properties-inl.h View 1 2 3 4 3 chunks +9 lines, -10 lines 0 comments Download
M src/compiler/scheduler.cc View 1 2 3 5 chunks +8 lines, -12 lines 0 comments Download
M src/compiler/simplified-lowering.cc View 1 2 3 4 5 6 7 2 chunks +11 lines, -13 lines 0 comments Download
M src/compiler/verifier.cc View 1 2 3 4 5 6 7 1 chunk +5 lines, -5 lines 0 comments Download
M test/cctest/compiler/test-control-reducer.cc View 1 2 3 4 5 6 1 chunk +2 lines, -5 lines 0 comments Download
M test/cctest/compiler/test-node.cc View 1 2 3 4 5 6 2 chunks +12 lines, -14 lines 0 comments Download

Messages

Total messages: 13 (4 generated)
danno
PTAL, wdyt?
6 years ago (2014-11-28 13:17:49 UTC) #2
Michael Starzinger
https://codereview.chromium.org/765983002/diff/40001/src/compiler/control-reducer.cc File src/compiler/control-reducer.cc (right): https://codereview.chromium.org/765983002/diff/40001/src/compiler/control-reducer.cc#newcode218 src/compiler/control-reducer.cc:218: for (auto edge : node->use_edges()) { Can we pretty ...
6 years ago (2014-11-28 13:55:46 UTC) #3
danno
Merged, feedback addressed. Please take another look.
6 years ago (2014-11-28 16:28:22 UTC) #4
Michael Starzinger
LGTM from my end. https://codereview.chromium.org/765983002/diff/110001/src/compiler/node.h File src/compiler/node.h (right): https://codereview.chromium.org/765983002/diff/110001/src/compiler/node.h#newcode27 src/compiler/node.h:27: class Edge; nit: Please alpha-sort.
6 years ago (2014-12-02 09:24:39 UTC) #5
danno
https://codereview.chromium.org/765983002/diff/110001/src/compiler/node.h File src/compiler/node.h (right): https://codereview.chromium.org/765983002/diff/110001/src/compiler/node.h#newcode27 src/compiler/node.h:27: class Edge; On 2014/12/02 09:24:39, Michael Starzinger wrote: > ...
6 years ago (2014-12-02 12:54:00 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/765983002/150001
6 years ago (2014-12-02 12:58:04 UTC) #8
commit-bot: I haz the power
Try jobs failed on following builders: v8_win64_rel on tryserver.v8 (http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel/builds/708)
6 years ago (2014-12-02 13:18:57 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/765983002/170001
6 years ago (2014-12-02 14:05:44 UTC) #12
commit-bot: I haz the power
6 years ago (2014-12-02 14:38:59 UTC) #13
Message was sent while issue was closed. 
Committed patchset #10 (id:170001)

Powered by Google App Engine
This is Rietveld 408576698