Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2883853002:
[turbofan] Always inline small functions directly. (Closed)
|
Created:
3 years, 7 months ago by Benedikt Meurer Modified:
3 years, 7 months ago Reviewers:
Jarin CC:
v8-reviews_googlegroups.com, Michael Hablich Target Ref:
refs/heads/master Project:
v8 Visibility:
Public. |
Description[turbofan] Always inline small functions directly.
Introduce a flag --max_inlined_nodes_small (defaults to 10), which gives
the upper limit of AST nodes for a function to be considered "small" by
the inlining heuristic. These functions will always be inlined
immediately, independent of the budget.
R=jarin@chromium.org
BUG=v8:6395, v8:6278, v8:6344, v8:6394
Review-Url: https://codereview.chromium.org/2883853002
Cr-Commit-Position: refs/heads/master@{#45291}
Committed: https://chromium.googlesource.com/v8/v8/+/558cee1d147947ab65fd3c99f9812093c6fc764a
Patch Set 1 #
Total comments: 4
Patch Set 2 : Check frequency #
Dependent Patchsets: Messages
Total messages: 25 (18 generated)
The CQ bit was checked by bmeurer@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm https://codereview.chromium.org/2883853002/diff/1/src/compiler/js-inlining-he... File src/compiler/js-inlining-heuristic.cc (right): https://codereview.chromium.org/2883853002/diff/1/src/compiler/js-inlining-he... src/compiler/js-inlining-heuristic.cc:68: bool SmallInlineFunction(Handle<SharedFunctionInfo> shared) { Could you rename to IsSmallInlineFunction? https://codereview.chromium.org/2883853002/diff/1/src/compiler/js-inlining-he... src/compiler/js-inlining-heuristic.cc:121: if (small_inline) { Could not this lead to infinite inlining? Perhaps it would be better to move this after the max-inlining-depth check...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: v8_win64_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng/builds/27475) v8_win64_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_win64_rel_ng_triggered/b...)
The CQ bit was checked by bmeurer@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2883853002/diff/1/src/compiler/js-inlining-he... File src/compiler/js-inlining-heuristic.cc (right): https://codereview.chromium.org/2883853002/diff/1/src/compiler/js-inlining-he... src/compiler/js-inlining-heuristic.cc:68: bool SmallInlineFunction(Handle<SharedFunctionInfo> shared) { On 2017/05/15 06:42:47, Jarin wrote: > Could you rename to IsSmallInlineFunction? Done. https://codereview.chromium.org/2883853002/diff/1/src/compiler/js-inlining-he... src/compiler/js-inlining-heuristic.cc:121: if (small_inline) { On 2017/05/15 06:42:47, Jarin wrote: > Could not this lead to infinite inlining? Perhaps it would be better to move > this after the max-inlining-depth check... Done.
The CQ bit was unchecked by bmeurer@chromium.org
The CQ bit was checked by bmeurer@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jarin@chromium.org Link to the patchset: https://codereview.chromium.org/2883853002/#ps20001 (title: "Address feedback.")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...) v8_linux64_gyp_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED, http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng_trigg...)
The CQ bit was checked by bmeurer@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by bmeurer@chromium.org
The CQ bit was checked by bmeurer@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from jarin@chromium.org Link to the patchset: https://codereview.chromium.org/2883853002/#ps40001 (title: "Check frequency")
Patchset #2 (id:20001) has been deleted
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1494834895916280,
"parent_rev": "e658bc575faa091c570a652b240e1da3866ee5c7", "commit_rev":
"558cee1d147947ab65fd3c99f9812093c6fc764a"}
Message was sent while issue was closed.
Description was changed from ========== [turbofan] Always inline small functions directly. Introduce a flag --max_inlined_nodes_small (defaults to 10), which gives the upper limit of AST nodes for a function to be considered "small" by the inlining heuristic. These functions will always be inlined immediately, independent of the budget. R=jarin@chromium.org BUG=v8:6395,v8:6278,v8:6344,v8:6394 ========== to ========== [turbofan] Always inline small functions directly. Introduce a flag --max_inlined_nodes_small (defaults to 10), which gives the upper limit of AST nodes for a function to be considered "small" by the inlining heuristic. These functions will always be inlined immediately, independent of the budget. R=jarin@chromium.org BUG=v8:6395,v8:6278,v8:6344,v8:6394 Review-Url: https://codereview.chromium.org/2883853002 Cr-Commit-Position: refs/heads/master@{#45291} Committed: https://chromium.googlesource.com/v8/v8/+/558cee1d147947ab65fd3c99f9812093c6f... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:40001) as https://chromium.googlesource.com/v8/v8/+/558cee1d147947ab65fd3c99f9812093c6f... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
