[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