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

Issue 2575703003: Move TraceInlinedFunction from Hydrogen graph builder to CompilationInfo. (Closed)

Created:
4 years ago by Vyacheslav Egorov (Google)
Modified:
4 years ago
CC:
v8-reviews_googlegroups.com
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Move TraceInlinedFunction from Hydrogen graph builder to internal::CodeGenerator. This allows to share source dumping infrastructure between CS and TF and opens a possibility for external tools like IRHydra to perform deoptimization to source mapping for TF generated code based on --trace-deopt --print-opt-code --code-comments output. This CL also restores an old TraceInlinedFunction behavior which was lost during source positions refactoring - originally TraceInlinedFunction dumped source code only once per-SFI to avoid large traces whenever some helper function is inlined multiple times. This CL also adds --print-opt-source flag that would in the future replace obsolete --hydrogen-track-positions. BUG= Review-Url: https://codereview.chromium.org/2575703003 Cr-Commit-Position: refs/heads/master@{#41758} Committed: https://chromium.googlesource.com/v8/v8/+/686d8c8685447c2b9e50b71106666b6d4e937871

Patch Set 1 #

Total comments: 3

Patch Set 2 : Address comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+99 lines, -68 lines) Patch
M src/codegen.cc View 1 1 chunk +92 lines, -0 lines 0 comments Download
M src/compiler/graph-visualizer.cc View 1 1 chunk +3 lines, -0 lines 0 comments Download
M src/crankshaft/hydrogen.h View 2 chunks +0 lines, -9 lines 0 comments Download
M src/crankshaft/hydrogen.cc View 3 chunks +0 lines, -59 lines 0 comments Download
M src/flag-definitions.h View 1 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 20 (10 generated)
Vyacheslav Egorov (Google)
PTAL
4 years ago (2016-12-14 15:48:12 UTC) #3
Benedikt Meurer
Tobias should review this, he did the source position support for TurboFan.
4 years ago (2016-12-14 17:41:06 UTC) #6
Tobias Tebbi
Thank you, IRHydra support for Turbofan would be awesome. https://codereview.chromium.org/2575703003/diff/1/src/compilation-info.cc File src/compilation-info.cc (right): https://codereview.chromium.org/2575703003/diff/1/src/compilation-info.cc#newcode222 src/compilation-info.cc:222: ...
4 years ago (2016-12-15 16:22:01 UTC) #7
Vyacheslav Egorov (Google)
Thanks for taking a look Tobias! Great idea about moving that to codegen. I think ...
4 years ago (2016-12-15 22:24:18 UTC) #8
Tobias Tebbi
On 2016/12/15 22:24:18, Vyacheslav Egorov (Google) wrote: > Thanks for taking a look Tobias! Great ...
4 years ago (2016-12-16 11:52:29 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2575703003/20001
4 years ago (2016-12-16 12:25:55 UTC) #12
commit-bot: I haz the power
No L-G-T-M from a valid reviewer yet. CQ run can only be started by full ...
4 years ago (2016-12-16 12:25:57 UTC) #14
Benedikt Meurer
LGTM (rubber-stamped)
4 years ago (2016-12-16 12:27:52 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2575703003/20001
4 years ago (2016-12-16 12:28:25 UTC) #17
commit-bot: I haz the power
4 years ago (2016-12-16 12:57:15 UTC) #20
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/v8/v8/+/686d8c8685447c2b9e50b71106666b6d4e9...

Powered by Google App Engine
This is Rietveld 408576698