[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