[PATCH] D22120: Move inline threshold related flags to InlineSimple.cpp

[Sean Silva via llvm-commits]

silvas added a subscriber: silvas.
silvas added a comment.

To clarify, this patch itself is just a NFC refactoring in preparation for the real change, right? (to prevent the existing cl::opt flags from interfering with preinline for instrumentation)

A couple nits inline.


================
Comment at: include/llvm/Analysis/InlineCost.h:122
@@ +121,3 @@
+  /// Threshold to use for cold callees.
+  Optional<int> ColdThreshold;
+  /// Threshold to use when the caller is optimized for size.
----------------
Can you add a comment about what happens when the "Optional" is in its "empty" state? I assume that `DefaultThreshold` is used in their place but it would be good to document explicitly what happens.

================
Comment at: lib/Analysis/InlineCost.cpp:593
@@ -615,1 +592,3 @@
 
+static int Min(int A, Optional<int> B) {
+  return B ? std::min(A, B.getValue()) : A;
----------------
This doesn't match the LLVM naming convention for functions (should start with lower case): http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

Also, you may want to make this 'min' function a local lambda in updateThreshold as it is only used there I think.


https://reviews.llvm.org/D22120