[PATCH] Switch to Select optimisations

David Majnemer david.majnemer at gmail.com
Mon Sep 29 10:02:08 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 {
----------------
Can you run clang-format on this?

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3449
@@ +3448,3 @@
+    }
+    CompareConstant(const Constant* c) : C(c) {}
+  };
----------------
LLVM convention is to have capitalized values.  Pointers and references should lean to the right.

================
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();
----------------
`for (SwitchInst::CaseIt I : SI->cases()` is a little more concise.

================
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()) {
----------------
I feel like this would be easier to follow if you used a lambda and captured `Result`.

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

================
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;
----------------
This looks wrong if the bitwidth of `Bits` exceeds an `i64`, consider using `APInt::getAllOnesValue` or `APInt::getMaxValue`.

http://reviews.llvm.org/D5525






More information about the llvm-commits mailing list