[PATCH] D17553: [SimplifyCFG] Speculatively flatten CFG based on profiling metadata (try 2)

[Sanjoy Das via llvm-commits]

sanjoy requested changes to this revision.
This revision now requires changes to proceed.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2708
@@ +2707,3 @@
+  /// Is the failing path of this branch taken rarely if at all?
+  auto isRarelyUntaken = [](BranchInst *BI) {
+    uint64_t ProbTrue;
----------------
Nit: should be `IsRarelyUntaken`.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2734
@@ +2733,3 @@
+    // Only flatten relatively small BBs to avoid making the bad case terrible
+    if (SpeculationCost > SpeculativeFlattenThreshold || isa<CallInst>(I))
+      return false;
----------------
Minor: might do this check before the check on `isSafeToSpeculativelyExecute`, since it will be a cheaper way out, if it fails.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2741
@@ +2740,3 @@
+  // PBI.  This can happen if (for instance), PBI ensures no overflow in the
+  // computaiton leading to BI's condition.  We can't use the overflow bits to
+  // prove the original condition checking for overflow.  This only considers
----------------
Spelling nit: "computation"

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2759
@@ +2758,3 @@
+    if (auto *OBO = dyn_cast<OverflowingBinaryOperator>(I))
+      if (OBO->hasNoUnsignedWrap() || OBO->hasNoSignedWrap())
+        // TODO: can we prove the flags are valid before PBI?
----------------
I'd say this is a little risky.  I think you're okay now since `isImpliedCondition` only looks at overflow flags, but if it is changed to consider, e.g., the `exact` flag on `udiv` or `inbounds` on `getelementptr` at some time in the future, this might subtly break.

I'd rather split out a helper routine, `GetCombinedCond` that takes two predicates, `P` and `Q` and a location, `Loc`, and returns a predicate that is equivalent to `P && Q` at `Loc`, if it can be cheaply computed.  Predicates like `5 u< L` and `6 u< L` can be combined into `6 u< L`; and predicates like `I u< L` and `(I +nuw 1) u< L` can be combined into `(I +nuw 1) u< L` if you can prove that the `nuw` holds at `Loc`, etc.


http://reviews.llvm.org/D17553