[PATCH] D73142: Heat Coloring for CFGPrinter and CallPrinter
Sean Fertile via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 31 07:31:09 PST 2020
sfertile added a comment.
I'm embarrassed to say I though the old patches landed after the Polly fix, so I really appreciate you taking the time to update the patches.
I've added a few comments to start/ One thing I would suggest is running clang-format over all the changes.
================
Comment at: llvm/include/llvm/Analysis/CFGPrinter.h:237
+ unsigned OpNo = I.getSuccessorIndex();
----------------
Minor nit: Fix the extra white space.
================
Comment at: llvm/include/llvm/Analysis/CFGPrinter.h:241
+ if (OpNo >= TI->getNumSuccessors())
+
return "";
----------------
Remove extra blank line.
================
Comment at: llvm/include/llvm/Analysis/CFGPrinter.h:250
+ ((double)BranchProb.getDenominator());
+ double Width = 1+(MaxEdgeWidth-1)*WeightPercent;
+
----------------
`const unsigned MaxEdgeWidth = 2`
so shouldn't this just be `double Width = 1.0 + WeightPercent;`
================
Comment at: llvm/include/llvm/Analysis/CFGPrinter.h:252
+
+ if (CFGInfo->useRawEdgeWeights()) {
+ // Prepend a 'W' to indicate that this is a weight rather than the actual
----------------
Can we convert the else block into an early return instead -->
```
if (!CFGInfo->useRawEdgeWeights())
return formatv("label=\"{0:P}\" penwidth={1}", WeightPercent, Width).str();
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73142/new/
https://reviews.llvm.org/D73142
More information about the llvm-commits
mailing list