[CaptureTracking] Replace hardcoded constant to option. NFC.

[skatkov]

Summary

The motivation is to be able to play with the option and change if it is required.

[fedor.sergeev]

llvm/include/llvm/Analysis/CaptureTracking.h

56–57

Flipping the order of two integer parameters is somewhat risky interface change, since it does not lead to errors or even warnings at old call-sites that pass values according to the old interface.

98

If you introduce this accessor function anyway, why dont you just specify it in all the default parameters.
Adding multiple overloads which just fill the default parameter looks too chatty.

[skatkov]

Handled Fedor's comments. Thank you for review.

[fedor.sergeev]

Look good to me, but I would like to hear other opinions...

llvm/lib/Analysis/CaptureTracking.cpp

32

I'm not sure if '-ct' is enough for people to figure out what it means.
I would mildly prefer -capture-tracking here....

[jdoerfert]

One nit, otherwise this seems fine to me

llvm/include/llvm/Analysis/CaptureTracking.h

24–27

Documentation please.

[skatkov]

llvm/include/llvm/Analysis/CaptureTracking.h

24–27

It is in cpp file. Would you like me to move it in hpp?

[jdoerfert]

@fedor.sergeev Feel free to accept this w/o waiting for me

llvm/include/llvm/Analysis/CaptureTracking.h

24–27

Now it looks almost fine. public functions should have documentation in the header.
Please reword it as it is a function and not a constant now. The stuff about "small"
can be removed or moved to the command line option.

[fedor.sergeev]

LGTM, thanks for cleaning this up!

llvm/include/llvm/Analysis/CaptureTracking.h

 namespace llvm {
 
   class Value;
   class Use;
   class DataLayout;
   class Instruction;
   class DominatorTree;
 
-  /// The default value for MaxUsesToExplore argument. It's relatively small to
-  /// keep the cost of analysis reasonable for clients like BasicAliasAnalysis,
-  /// where the results can't be cached.
-  /// TODO: we should probably introduce a caching CaptureTracking analysis and
-  /// use it where possible. The caching version can use much higher limit or
-  /// don't have this cap at all.
-  unsigned constexpr DefaultMaxUsesToExplore = 20;
+  /// getDefaultMaxUsesToExploreForCaptureTracking - Return default value of
+  /// the maximal number of uses to explore before giving up. It is used by
+  /// PointerMayBeCaptured family analysys.
+  unsigned getDefaultMaxUsesToExploreForCaptureTracking();
 
   /// PointerMayBeCaptured - Return true if this pointer value may be captured
   /// by the enclosing function (which is required to exist).  This routine can
   /// be expensive, so consider caching the results.  The boolean ReturnCaptures
   /// specifies whether returning the value (or part of it) from the function
   /// counts as capturing it or not.  The boolean StoreCaptures specified
   /// whether storing the value (or part of it) into memory anywhere
   /// automatically counts as capturing it or not.
   /// MaxUsesToExplore specifies how many uses should the analysis explore for
   /// one value before giving up due too "too many uses".
-  bool PointerMayBeCaptured(const Value *V,
-                            bool ReturnCaptures,
+  bool PointerMayBeCaptured(const Value *V, bool ReturnCaptures,
                             bool StoreCaptures,
-                            unsigned MaxUsesToExplore = DefaultMaxUsesToExplore);
+                            unsigned MaxUsesToExplore =
+                                getDefaultMaxUsesToExploreForCaptureTracking());
 
   /// PointerMayBeCapturedBefore - Return true if this pointer value may be
   /// captured by the enclosing function (which is required to exist). If a
   /// DominatorTree is provided, only captures which happen before the given
   /// instruction are considered. This routine can be expensive, so consider
   /// caching the results.  The boolean ReturnCaptures specifies whether
   /// returning the value (or part of it) from the function counts as capturing
   /// it or not.  The boolean StoreCaptures specified whether storing the value
   /// (or part of it) into memory anywhere automatically counts as capturing it
   /// or not. Captures by the provided instruction are considered if the
   /// final parameter is true.
   /// MaxUsesToExplore specifies how many uses should the analysis explore for
   /// one value before giving up due too "too many uses".
-  bool PointerMayBeCapturedBefore(const Value *V, bool ReturnCaptures,
-                                  bool StoreCaptures, const Instruction *I,
-                                  const DominatorTree *DT, bool IncludeI = false,
-                                  unsigned MaxUsesToExplore = DefaultMaxUsesToExplore);
+  bool PointerMayBeCapturedBefore(
+      const Value *V, bool ReturnCaptures, bool StoreCaptures,
+      const Instruction *I, const DominatorTree *DT, bool IncludeI = false,
+      unsigned MaxUsesToExplore =
+          getDefaultMaxUsesToExploreForCaptureTracking());
 
   /// This callback is used in conjunction with PointerMayBeCaptured. In
   /// addition to the interface here, you'll need to provide your own getters
   /// to see whether anything was captured.
   struct CaptureTracker {
     virtual ~CaptureTracker();
 
     /// tooManyUses - The depth of traversal has breached a limit. There may be
namespace llvm {
   };
 
   /// PointerMayBeCaptured - Visit the value and the values derived from it and
   /// find values which appear to be capturing the pointer value. This feeds
   /// results into and is controlled by the CaptureTracker object.
   /// MaxUsesToExplore specifies how many uses should the analysis explore for
   /// one value before giving up due too "too many uses".
   void PointerMayBeCaptured(const Value *V, CaptureTracker *Tracker,
-                            unsigned MaxUsesToExplore = DefaultMaxUsesToExplore);
+                            unsigned MaxUsesToExplore =
+                                getDefaultMaxUsesToExploreForCaptureTracking());
 } // end namespace llvm
 
 #endif

llvm/lib/Analysis/CaptureTracking.cpp

 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
 
 using namespace llvm;
 
+/// The default value for MaxUsesToExplore argument. It's relatively small to
+/// keep the cost of analysis reasonable for clients like BasicAliasAnalysis,
+/// where the results can't be cached.
+/// TODO: we should probably introduce a caching CaptureTracking analysis and
+/// use it where possible. The caching version can use much higher limit or
+/// don't have this cap at all.
+static cl::opt<unsigned>
+DefaultMaxUsesToExplore("capture-tracking-max-uses-to-explore", cl::Hidden,
+                        cl::desc("Maximal number of uses to explore."),
+                        cl::init(20));
+
+unsigned llvm::getDefaultMaxUsesToExploreForCaptureTracking() {
+  return DefaultMaxUsesToExplore;
+}
+
 CaptureTracker::~CaptureTracker() {}
 
 bool CaptureTracker::shouldExplore(const Use *U) { return true; }
 
 bool CaptureTracker::isDereferenceableOrNull(Value *O, const DataLayout &DL) {
   // An inbounds GEP can either be a valid pointer (pointing into
   // or to the end of an allocation), or be null in the default
   // address space. So for an inbounds GEP there is no way to let
bool llvm::PointerMayBeCapturedBefore(const Value *V, bool ReturnCaptures,
   CapturesBefore CB(ReturnCaptures, I, DT, IncludeI);
   PointerMayBeCaptured(V, &CB, MaxUsesToExplore);
   return CB.Captured;
 }
 
 void llvm::PointerMayBeCaptured(const Value *V, CaptureTracker *Tracker,
                                 unsigned MaxUsesToExplore) {
   assert(V->getType()->isPointerTy() && "Capture is for pointers only!");
-  SmallVector<const Use *, DefaultMaxUsesToExplore> Worklist;
-  SmallSet<const Use *, DefaultMaxUsesToExplore> Visited;
+  SmallVector<const Use *, 20> Worklist;
+  Worklist.reserve(getDefaultMaxUsesToExploreForCaptureTracking());
+  SmallSet<const Use *, 20> Visited;
 
   auto AddUses = [&](const Value *V) {
     unsigned Count = 0;
     for (const Use &U : V->uses()) {
       // If there are lots of uses, conservatively say that the value
       // is captured to avoid taking too much compile time.
       if (Count++ >= MaxUsesToExplore)
         return Tracker->tooManyUses();

llvm/lib/Transforms/IPO/AttributorAttributes.cpp

if (IsVoidTy || CheckReturnedArgs(*RVAA)) {
       }
     }
   }
 
   // Use the CaptureTracker interface and logic with the specialized tracker,
   // defined in AACaptureUseTracker, that can look at in-flight abstract
   // attributes and directly updates the assumed state.
   SmallVector<const Value *, 4> PotentialCopies;
-  unsigned RemainingUsesToExplore = DefaultMaxUsesToExplore;
+  unsigned RemainingUsesToExplore =
+      getDefaultMaxUsesToExploreForCaptureTracking();
   AACaptureUseTracker Tracker(A, *this, IsDeadAA, T, PotentialCopies,
                               RemainingUsesToExplore);
 
   // Check all potential copies of the associated value until we can assume
   // none will be captured or we have to assume at least one might be.
   unsigned Idx = 0;
   PotentialCopies.push_back(V);
   while (T.isAssumed(NO_CAPTURE_MAYBE_RETURNED) && Idx < PotentialCopies.size())