[PATCH] D13697: [SimplifyCFG] Merge conditional stores

[James Molloy via llvm-commits]

jmolloy marked 5 inline comments as done.

================
Comment at: include/llvm/ADT/SetOperations.h:42
@@ -41,3 +41,3 @@
    for (typename S1Ty::iterator I = S1.begin(); I != S1.end();) {
-     const typename S1Ty::key_type &E = *I;
+     const auto &E = *I;
      ++I;
----------------
This was required because SmallPtrSet doesn't have ::key_type. auto is obviously a better way to do this. I'll obviously apply this as a separate commit.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2416
@@ +2415,3 @@
+
+  // Now check the stores are compatible.
+  if (QStore->getPointerOperand()->getType() != PStore->getPointerOperand()->getType() ||
----------------
Ouch! good catch, thanks!

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2517
@@ +2516,3 @@
+  //
+  // This can reduce the number of stores executed if both of the conditions are
+  // true, and can allow the blocks to become small enough to be if-converted.
----------------
In that case, yes it is. The important part is "if the code doesn't simplify further" though - even if both conditions are always false, if we can if-convert this is very likely to be a win.

The heuristics at the moment are trying to catch cases where we know we can if-convert.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2662
@@ -2387,3 +2661,3 @@
   }
 
   if (auto *CE = dyn_cast<ConstantExpr>(BI->getCondition()))
----------------
I've been selectively running clang-format on the bits I've touched, and missed this bit :(

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:5101
@@ -4802,3 +5100,3 @@
 ///
 bool llvm::SimplifyCFG(BasicBlock *BB, const TargetTransformInfo &TTI,
                        unsigned BonusInstThreshold, AssumptionCache *AC) {
----------------
Yes, this was me undoing a change and blindingly running clang-format. More below, where clang-format has ripped trailing whitespace away :(


Repository:
  rL LLVM

http://reviews.llvm.org/D13697