[PATCH] Switch to Select optimisations - Two Case switch to select
hayarms at gmail.com
Mon Oct 6 20:10:37 PDT 2014
I do have commit access :)
Let me know if you think it is better removing function names from the comments or not considering most of the functions have them, and after addressing this last thing I will commit it.
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."
I put the name in the comment, because most of the other functions in the file follow that style.
I thought it was a good idea to follow the general style in the file.
As far as "This is ..." is concerned I think you are right, that piece is redundant.
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.
More information about the llvm-commits