Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1547743002:
doxygenize IceClFlags.cpp (Closed)
|
Created:
4 years, 12 months ago by rkotlerimgtec Modified:
4 years, 11 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. |
Descriptiondoxygenize IceClFlags.cpp
This is a first cut at this. More can be done and much of this is just moving what is already in the help for the
commands into Doxygen comments. The documentation can be
expanded to better describe the role of the various
command line options.
BUG=
R=stichnot@chromium.org
Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=2233987b85c2de6cc31f5f510a8513d555cdb3bc
Patch Set 1 #
Total comments: 40
Patch Set 2 : changes suggested by stichnot #Patch Set 3 : run clang format #Messages
Total messages: 11 (3 generated)
Description was changed from ========== doxygenize IceClFlags.cpp in progress BUG= ========== to ========== doxygenize IceClFlags.cpp This is a first cut at this. More can be done and much of this is just moving what is already in the help for the commands into Doxygen comments. The documentation can be expanded to better describe the role of the various command line options. BUG= ==========
rkotlerimgtec@gmail.com changed reviewers: + jpp@chromium.org, kschimpf@chromium.org, stichnot@chromium.org
https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp File src/IceClFlags.cpp (right): https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode47 src/IceClFlags.cpp:47: /// Alias for --allow-externally-defined-symbols End with a period for consistency. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode61 src/IceClFlags.cpp:61: /// crosstest.py. I would say "... by the cross tests", to give better future-proofing against the crosstest.py filename changing. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode99 src/IceClFlags.cpp:99: /// Disable Subzero translation End with a period for consistency. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode115 src/IceClFlags.cpp:115: /// If true, instrument basic blocks, and output profiling information to I would remove the "If true, " intro, in the comment and the help string. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode148 src/IceClFlags.cpp:148: /// Optimization level Om1, O-1, O0, O0, O1, O2 End with a period for consistency. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode222 src/IceClFlags.cpp:222: /// Target architecture attributes". remove the quote https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode238 src/IceClFlags.cpp:238: /// Prepend a prefix to symbol names for testing End with a period for consistency. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode287 src/IceClFlags.cpp:287: /// Verbose options (can be comma-separated): Replace ':' with '.' . https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode316 src/IceClFlags.cpp:316: /// Exit with success status, even if errors found End with a period for consistency. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode323 src/IceClFlags.cpp:323: /// build. We double check that this flag at runtime to make sure the Remove "that" ? https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode330 src/IceClFlags.cpp:330: /// Define format of input file: Replace ':' with '.' . https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode344 src/IceClFlags.cpp:344: /// <IR file> Probably better to call this "Input file." https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode348 src/IceClFlags.cpp:348: /// Set log filename End with a period for consistency. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode353 src/IceClFlags.cpp:353: /// IR first End with a period for consistency. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode364 src/IceClFlags.cpp:364: /// Define the command line options for immediates pooling and randomization End with a period for consistency. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode385 src/IceClFlags.cpp:385: /// Command line option for turning on basic block shuffling. Change this comment to match cl::desc, here and below. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode388 src/IceClFlags.cpp:388: cl::desc("Shuffle the layout of basic blocks in each functions"), s/functions/function/ https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode394 src/IceClFlags.cpp:394: cl::desc("Reorder the layout of functions in TEXT section"), maybe just "Randomize function ordering." https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode408 src/IceClFlags.cpp:408: cl::desc("Reorder the layout of global variables in NON TEXT section"), maybe just "Randomize global data ordering." https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode414 src/IceClFlags.cpp:414: cl::desc("Reorder the layout of constants in constant pools"), maybe just "Randomize constant pool entry ordering."
https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp File src/IceClFlags.cpp (right): https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode47 src/IceClFlags.cpp:47: /// Alias for --allow-externally-defined-symbols On 2015/12/23 16:23:54, stichnot wrote: > End with a period for consistency. Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode61 src/IceClFlags.cpp:61: /// crosstest.py. On 2015/12/23 16:23:54, stichnot wrote: > I would say "... by the cross tests", to give better future-proofing against the > crosstest.py filename changing. Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode99 src/IceClFlags.cpp:99: /// Disable Subzero translation On 2015/12/23 16:23:54, stichnot wrote: > End with a period for consistency. Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode115 src/IceClFlags.cpp:115: /// If true, instrument basic blocks, and output profiling information to On 2015/12/23 16:23:54, stichnot wrote: > I would remove the "If true, " intro, in the comment and the help string. Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode148 src/IceClFlags.cpp:148: /// Optimization level Om1, O-1, O0, O0, O1, O2 On 2015/12/23 16:23:54, stichnot wrote: > End with a period for consistency. Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode222 src/IceClFlags.cpp:222: /// Target architecture attributes". On 2015/12/23 16:23:54, stichnot wrote: > remove the quote Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode238 src/IceClFlags.cpp:238: /// Prepend a prefix to symbol names for testing On 2015/12/23 16:23:54, stichnot wrote: > End with a period for consistency. Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode287 src/IceClFlags.cpp:287: /// Verbose options (can be comma-separated): On 2015/12/23 16:23:54, stichnot wrote: > Replace ':' with '.' . Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode316 src/IceClFlags.cpp:316: /// Exit with success status, even if errors found On 2015/12/23 16:23:54, stichnot wrote: > End with a period for consistency. Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode323 src/IceClFlags.cpp:323: /// build. We double check that this flag at runtime to make sure the On 2015/12/23 16:23:54, stichnot wrote: > Remove "that" ? Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode330 src/IceClFlags.cpp:330: /// Define format of input file: On 2015/12/23 16:23:54, stichnot wrote: > Replace ':' with '.' . Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode344 src/IceClFlags.cpp:344: /// <IR file> On 2015/12/23 16:23:54, stichnot wrote: > Probably better to call this "Input file." Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode348 src/IceClFlags.cpp:348: /// Set log filename On 2015/12/23 16:23:54, stichnot wrote: > End with a period for consistency. Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode353 src/IceClFlags.cpp:353: /// IR first On 2015/12/23 16:23:54, stichnot wrote: > End with a period for consistency. Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode364 src/IceClFlags.cpp:364: /// Define the command line options for immediates pooling and randomization On 2015/12/23 16:23:54, stichnot wrote: > End with a period for consistency. Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode385 src/IceClFlags.cpp:385: /// Command line option for turning on basic block shuffling. On 2015/12/23 16:23:54, stichnot wrote: > Change this comment to match cl::desc, here and below. Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode388 src/IceClFlags.cpp:388: cl::desc("Shuffle the layout of basic blocks in each functions"), On 2015/12/23 16:23:54, stichnot wrote: > s/functions/function/ Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode394 src/IceClFlags.cpp:394: cl::desc("Reorder the layout of functions in TEXT section"), On 2015/12/23 16:23:54, stichnot wrote: > maybe just "Randomize function ordering." Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode408 src/IceClFlags.cpp:408: cl::desc("Reorder the layout of global variables in NON TEXT section"), On 2015/12/23 16:23:54, stichnot wrote: > maybe just "Randomize global data ordering." Done. https://codereview.chromium.org/1547743002/diff/1/src/IceClFlags.cpp#newcode414 src/IceClFlags.cpp:414: cl::desc("Reorder the layout of constants in constant pools"), On 2015/12/23 16:23:54, stichnot wrote: > maybe just "Randomize constant pool entry ordering." Done.
lgtm
On 2015/12/23 23:06:45, stichnot wrote: > lgtm Actually, not lgtm quite yet... looks like you forgot to "make format"
lgtm
Description was changed from ========== doxygenize IceClFlags.cpp This is a first cut at this. More can be done and much of this is just moving what is already in the help for the commands into Doxygen comments. The documentation can be expanded to better describe the role of the various command line options. BUG= ========== to ========== doxygenize IceClFlags.cpp This is a first cut at this. More can be done and much of this is just moving what is already in the help for the commands into Doxygen comments. The documentation can be expanded to better describe the role of the various command line options. BUG= 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 2233987b85c2de6cc31f5f510a8513d555cdb3bc (presubmit successful). |
