Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2217773003:
Documentation for LCSE, LICM, Short-Circuit, Global-Splitting (Closed)
|
Created:
4 years, 4 months ago by manasijm Modified:
4 years, 4 months ago CC:
native-client-reviews_googlegroups.com Base URL:
https://chromium.googlesource.com/native_client/pnacl-subzero.git@master Target Ref:
refs/heads/master Visibility:
Public. |
DescriptionDocumentation for LCSE, LICM, Short-Circuit, Global-Splitting
LCSE is local common sub-expression elimination.
LICM is loop invariant code motion.
Short circuit splits basic blocks and introduces early jumps.
Global Splitting is a post regalloc live range splitting pass.
BUG=none
R=stichnot@chromium.org
Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=a41e9a1432e58d9421e09cbb4d9f33585b240e56
Patch Set 1 #
Total comments: 29
Patch Set 2 : Address comments #
Total comments: 1
Patch Set 3 : Remove extra line #Messages
Total messages: 10 (3 generated)
manasijm@google.com changed reviewers: + eholk@chromium.org, jpp@chromium.org, kschimpf@google.com, stichnot@chromium.org
Description was changed from ========== Documentaion for LCSE, LICM, Short-Circuit, Global-Splitting LCSE is local common sub-expression elimination. LICM is loop invariant code motion. Short circuit splits basic blocks and introduces early jumps. Global Splitting is a post regalloc live range splitting pass. BUG=none ========== to ========== Documentation for LCSE, LICM, Short-Circuit, Global-Splitting LCSE is local common sub-expression elimination. LICM is loop invariant code motion. Short circuit splits basic blocks and introduces early jumps. Global Splitting is a post regalloc live range splitting pass. BUG=none ==========
Typo, title should be 'Documentation for LCSE, LICM, Short-Circuit, Global-Splitting' On Thu, Aug 4, 2016 at 6:19 PM, <manasijm@google.com> wrote: > Reviewers: stichnot, John, Eric Holk, Karl > CL: https://codereview.chromium.org/2217773003/ > > Description: > Documentaion for LCSE, LICM, Short-Circuit, Global-Splitting > > LCSE is local common sub-expression elimination. > LICM is loop invariant code motion. > Short circuit splits basic blocks and introduces early jumps. > Global Splitting is a post regalloc live range splitting pass. > > BUG=none > > Base URL: https://chromium.googlesource.com/native_client/pnacl- > subzero.git@master > > Affected files (+100, -0 lines): > M docs/DESIGN.rst > > > -- You received this message because you are subscribed to the Google Groups "Native-Client-Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to native-client-reviews+unsubscribe@googlegroups.com. To post to this group, send email to native-client-reviews@googlegroups.com. Visit this group at https://groups.google.com/group/native-client-reviews. For more options, visit https://groups.google.com/d/optout.
https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst File docs/DESIGN.rst (right): https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode59 docs/DESIGN.rst:59: | Local common sub-exp elimination | | I think you can spell out "subexpression" and (painfully, sorry) expand the columns, and still keep it with 80-col. https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode523 docs/DESIGN.rst:523: - Local common sub-expression elimination "Subexpression" is usually written without the hyphen. E.g. https://en.wikipedia.org/wiki/Common_subexpression_elimination https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode708 docs/DESIGN.rst:708: The Local CSE implementation goes through each instruction and record ``Seen`` records https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode709 docs/DESIGN.rst:709: instructions in a ``CfgUnorderedSet``. The equality comparator for this I don't think the level of detail in this document ever talks about specific Subzero data structure names. How about something like this? The Local CSE implementation goes through each instruction and records a portion of each ``Seen`` instruction in a hashset-like container. That portion consists of the entire instruction except for any dest variable. That means... https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode724 docs/DESIGN.rst:724: supported with the -lcse=no-ssa option. This has to maintain some extra ``-lcse=no-ssa`` https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode730 docs/DESIGN.rst:730: Loop invariant code motion I think it's more common to hyphenate "loop-invariant" here. https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode739 docs/DESIGN.rst:739: invariant instructions converge. In each iteration, a non-invariant instruction converges https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode744 docs/DESIGN.rst:744: Improving the loop-analysis infrastructure is likely to have significant effect maybe s/effect/impact/ ? https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode746 docs/DESIGN.rst:746: header has multiple incoming edges from outside could also be good idea. be a good https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode750 docs/DESIGN.rst:750: Short Circuit Evaluation For consistency, only capitalize the first word of the section. https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode753 docs/DESIGN.rst:753: Short Circuit Evaluation splits nodes and introduces early jumps when the result I would also not capitalize all these words. To set it apart from the rest of the text, consider using italics? https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode754 docs/DESIGN.rst:754: of a logical operation can be determined early and there are no side effects of maybe "no observable side effects" ? https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode757 docs/DESIGN.rst:757: When a definition of a variable involved in a conditional jump is found, an reflow paragraph, or make it a new paragraph https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode787 docs/DESIGN.rst:787: Global variable splitting (-split-global-vars) is run after register allocation. ``-split-global-vars``
https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst File docs/DESIGN.rst (right): https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode59 docs/DESIGN.rst:59: | Local common sub-exp elimination | | On 2016/08/05 06:20:00, stichnot wrote: > I think you can spell out "subexpression" and (painfully, sorry) expand the > columns, and still keep it with 80-col. Won't be that painful! I might not be an emacs ninja, but kdevelop/kate has picked up some useful features. https://kate-editor.org/2013/09/09/multi-line-text-editing-in-kate/
https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst File docs/DESIGN.rst (right): https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode59 docs/DESIGN.rst:59: | Local common sub-exp elimination | | On 2016/08/05 06:57:09, manasijm wrote: > On 2016/08/05 06:20:00, stichnot wrote: > > I think you can spell out "subexpression" and (painfully, sorry) expand the > > columns, and still keep it with 80-col. > > Won't be that painful! > I might not be an emacs ninja, but kdevelop/kate has picked up some useful > features. > https://kate-editor.org/2013/09/09/multi-line-text-editing-in-kate/ Done. https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode523 docs/DESIGN.rst:523: - Local common sub-expression elimination On 2016/08/05 06:20:00, stichnot wrote: > "Subexpression" is usually written without the hyphen. > E.g. https://en.wikipedia.org/wiki/Common_subexpression_elimination Done. https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode708 docs/DESIGN.rst:708: The Local CSE implementation goes through each instruction and record ``Seen`` On 2016/08/05 06:20:00, stichnot wrote: > records Done. https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode709 docs/DESIGN.rst:709: instructions in a ``CfgUnorderedSet``. The equality comparator for this On 2016/08/05 06:20:00, stichnot wrote: > I don't think the level of detail in this document ever talks about specific > Subzero data structure names. How about something like this? > > The Local CSE implementation goes through each instruction and records a portion > of each ``Seen`` instruction in a hashset-like container. That portion consists > of the entire instruction except for any dest variable. That means... Done. https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode724 docs/DESIGN.rst:724: supported with the -lcse=no-ssa option. This has to maintain some extra On 2016/08/05 06:19:59, stichnot wrote: > ``-lcse=no-ssa`` Done. https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode730 docs/DESIGN.rst:730: Loop invariant code motion On 2016/08/05 06:20:00, stichnot wrote: > I think it's more common to hyphenate "loop-invariant" here. Done. https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode739 docs/DESIGN.rst:739: invariant instructions converge. In each iteration, a non-invariant instruction On 2016/08/05 06:20:00, stichnot wrote: > converges Done. https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode744 docs/DESIGN.rst:744: Improving the loop-analysis infrastructure is likely to have significant effect On 2016/08/05 06:20:00, stichnot wrote: > maybe s/effect/impact/ ? Done. https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode746 docs/DESIGN.rst:746: header has multiple incoming edges from outside could also be good idea. On 2016/08/05 06:20:00, stichnot wrote: > be a good Done. https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode750 docs/DESIGN.rst:750: Short Circuit Evaluation On 2016/08/05 06:20:00, stichnot wrote: > For consistency, only capitalize the first word of the section. Done. https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode753 docs/DESIGN.rst:753: Short Circuit Evaluation splits nodes and introduces early jumps when the result On 2016/08/05 06:19:59, stichnot wrote: > I would also not capitalize all these words. To set it apart from the rest of > the text, consider using italics? Done. https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode754 docs/DESIGN.rst:754: of a logical operation can be determined early and there are no side effects of On 2016/08/05 06:20:00, stichnot wrote: > maybe "no observable side effects" ? Done. https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode757 docs/DESIGN.rst:757: When a definition of a variable involved in a conditional jump is found, an On 2016/08/05 06:20:00, stichnot wrote: > reflow paragraph, or make it a new paragraph Done. https://codereview.chromium.org/2217773003/diff/1/docs/DESIGN.rst#newcode787 docs/DESIGN.rst:787: Global variable splitting (-split-global-vars) is run after register allocation. On 2016/08/05 06:19:59, stichnot wrote: > ``-split-global-vars`` Done.
lgtm https://codereview.chromium.org/2217773003/diff/20001/docs/DESIGN.rst File docs/DESIGN.rst (right): https://codereview.chromium.org/2217773003/diff/20001/docs/DESIGN.rst#newcode800 docs/DESIGN.rst:800: Just a single blank line for consistency?
Description was changed from ========== Documentation for LCSE, LICM, Short-Circuit, Global-Splitting LCSE is local common sub-expression elimination. LICM is loop invariant code motion. Short circuit splits basic blocks and introduces early jumps. Global Splitting is a post regalloc live range splitting pass. BUG=none ========== to ========== Documentation for LCSE, LICM, Short-Circuit, Global-Splitting LCSE is local common sub-expression elimination. LICM is loop invariant code motion. Short circuit splits basic blocks and introduces early jumps. Global Splitting is a post regalloc live range splitting pass. BUG=none R=stichnot@chromium.org Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) manually as a41e9a1432e58d9421e09cbb4d9f33585b240e56 (presubmit successful). |
