[PATCH] Switch to Select optimisations

Marcello Maggioni hayarms at gmail.com
Mon Sep 29 10:36:31 PDT 2014


================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3438-3443
@@ +3437,8 @@
+// there is not a common destination block for the switch.
+static bool InitializeUniqueCases(
+    SwitchInst *SI, const DataLayout *DL, PHINode *&PHI,
+    BasicBlock *&CommonDest,
+    SmallVectorImpl<std::pair<Constant *, SmallVector<ConstantInt *, 4>>>
+    &UniqueResults,
+    Constant *&DefaultResult) {
+  struct CompareConstant {
----------------
majnemer wrote:
> Can you run clang-format on this?
Done.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3449
@@ +3448,3 @@
+    }
+    CompareConstant(const Constant* c) : C(c) {}
+  };
----------------
majnemer wrote:
> LLVM convention is to have capitalized values.  Pointers and references should lean to the right.
Removed because of using the Lambda syntax you suggested

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3451
@@ +3450,3 @@
+  };
+  for (auto I = SI->case_begin(), E = SI->case_end(); I != E; ++I) {
+    ConstantInt *CaseVal = I.getCaseValue();
----------------
majnemer wrote:
> `for (SwitchInst::CaseIt I : SI->cases()` is a little more concise.
Done

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3472-3473
@@ +3471,4 @@
+    auto &Result = *Results.begin();
+    auto ResultFound = std::find_if(UniqueResults.begin(), UniqueResults.end(),
+        CompareConstant(Result.second));
+    if (ResultFound == UniqueResults.end()) {
----------------
majnemer wrote:
> I feel like this would be easier to follow if you used a lambda and captured `Result`.
Done.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3738
@@ +3737,3 @@
+      DefaultResult, Cond, Builder);
+  if (SelectValue != nullptr) {
+    RemoveSwitchAfterSelectConversion(SI, PHI, SelectValue, CommonDest,
----------------
majnemer wrote:
> This would be more concise as `if (SelectValue)`
Done

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3747-3748
@@ +3746,4 @@
+  bool DefaultCanTrigger;
+  APInt KnownZeroFirst(Bits, ~(0ULL)), KnownOneFirst(Bits, ~(0ULL)),
+      KnownZeroSecond(Bits, ~(0ULL)), KnownOneSecond(Bits, ~(0ULL));
+  unsigned FirstPossibleHits = 0, SecondPossibleHits = 0;
----------------
majnemer wrote:
> This looks wrong if the bitwidth of `Bits` exceeds an `i64`, consider using `APInt::getAllOnesValue` or `APInt::getMaxValue`.
Done, thanks for this :)

http://reviews.llvm.org/D5525






More information about the llvm-commits mailing list