[PATCH] D21183: Better selection of common base address in constant hoisting

[Mehdi AMINI via llvm-commits]

mehdi_amini added a comment.

A few very superficial comments.


================
Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:328
@@ +327,3 @@
+// function only when we're optimising for size and there are less than 100
+// constants, we fall back to the old algorithm otherwise.
+unsigned
----------------
Thanks very much for well documenting this!

================
Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:390
@@ +389,3 @@
+        MaxCostItr = ConstCand;
+      }
+    }
----------------
Any reason to add braces here?

================
Comment at: lib/Transforms/Scalar/ConstantHoisting.cpp:392
@@ -299,2 +391,3 @@
+    }
   }
 
----------------
All this code could be sunk in `maximizeConstantsInRange()`.
It would also make the comment that described `maximizeConstantsInRange` more correct (it mentions the 100 limits and falling back to the old algorithm).


http://reviews.llvm.org/D21183