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

[Philip Reames via llvm-commits]

reames added inline comments.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2353
@@ +2352,3 @@
+/// path.  This is code size neutral, but makes it dynamically more expensive
+/// to fail either check.  As such, we only want to do this if both checks are
+/// expected to essentially never fail.
----------------
hfinkel wrote:
> either check -> any of the checks
Happy to make the requested change, but given this applies to a pair of branches, not quite clear why you think the new wording is more clear.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2382
@@ +2381,3 @@
+      return false;
+    return ProbTrue > ProbFalse * 100;
+  };
----------------
hfinkel wrote:
> I'd rather that this 100 ratio be a command-line parameter for easier experimentation.
Will do.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2403
@@ +2402,3 @@
+    // Only flatten relatively small BBs to avoid making the bad case terrible
+    if (SpeculationCost > 10 || isa<CallInst>(I) || isa<InvokeInst>(I))
+      return false;
----------------
hfinkel wrote:
> 10 should be a command-line parameter too.
Will do.


http://reviews.llvm.org/D13070