[PATCH] D13070: [SimplifyCFG] Speculatively flatten CFG based on profiling metadata

[Sanjoy Das via llvm-commits]

sanjoy added inline comments.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2371
@@ +2370,3 @@
+/// from BI->getParent() down both paths
+static bool TryToSpeculativelyFlattenCondBranchToCondBranch(BranchInst *PBI, BranchInst *BI,
+                                                            const DataLayout &DL) {
----------------
I'd drop the `TryTo`.  LLVM usually names such functions directly (`InlineFunction`, `UnrollLoop` etc.).

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2394
@@ +2393,3 @@
+
+  // Can we speculate everything in the given block (except for the terminator
+  // instruction) at the instruction boundary before 'At'?
----------------
Why is this needed?  Since you're changing `PBI`'s condition to be `BI`'s condition, and `BI`'s condition implies `PBI`'s condition, I don't see what gets speculatively executed.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2493
@@ -2390,1 +2492,3 @@
 
+  if (ConstantExpr *CE = dyn_cast<ConstantExpr>(BI->getCondition()))
+    if (CE->canTrap())
----------------
`auto *` is clearer, since the type is obvious from the `dyn_cast`.


http://reviews.llvm.org/D13070