[PATCH] Switch to Select optimisations - Two Case switch to select

Hans Wennborg hans at chromium.org
Mon Oct 6 11:00:35 PDT 2014


Thanks for breaking this out to a separate patch! It makes it much easier to review.

I think this is ready to be committed with the comments below addressed. Do you have commit access, or would you like me to land it for you?

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3453
@@ -3442,1 +3452,3 @@
 
+// MapCaseToResult - This is an helper function used to
+// add CaseVal to the list of cases that generate Result.
----------------
nit: there is no need to repeat the function name in the comment. This goes for the other functions too.

And instead of "This is an helper function used to..." I'd just put "Helper function that adds CaseVal to the list of cases that generate Result."

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:3602
@@ +3601,3 @@
+  // Compute known bits of the Condition of the Switch statement.
+  computeKnownBits(Cond, KnownZerosCond, KnownOnesCond, DL, 0, AT, SI);
+
----------------
The lines from "const unsigned BitWidth" to "computeKnownBits(...)" look like leftovers from the other patch.

================
Comment at: test/Transforms/SimplifyCFG/switch-to-select-two-case.ll:15
@@ +14,3 @@
+; CHECK-LABEL: @foo1_with_default
+; CHECK: icmp eq i32
+; CHECK-NEXT: %switch.select = select i1 %switch.selectcmp, i32 2, i32 4
----------------
Please check the full icmp instruction, i.e. capture the name and what is being compared.

================
Comment at: test/Transforms/SimplifyCFG/switch-to-select-two-case.ll:51
@@ +50,3 @@
+; CHECK-LABEL: @foo1_without_default
+; CHECK: icmp eq i32
+; CHECK-NEXT: %switch.select = select i1 %switch.selectcmp, i32 10, i32 2
----------------
Please check the full icmp instruction.

http://reviews.llvm.org/D5620






More information about the llvm-commits mailing list