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

Marcello Maggioni 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.

Thanks again!

================
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.
----------------
hans wrote:
> 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);
+
----------------
hans wrote:
> The lines from "const unsigned BitWidth" to "computeKnownBits(...)" look like leftovers from the other patch.
Whoops :D

================
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
----------------
hans wrote:
> Please check the full icmp instruction, i.e. capture the name and what is being compared.
Done

http://reviews.llvm.org/D5620






More information about the llvm-commits mailing list