[PATCH] D78624: [CaptureTracking] Replace hardcoded constant to option. NFC.

Johannes Doerfert via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 22 10:52:54 PDT 2020


jdoerfert added a comment.

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



================
Comment at: llvm/include/llvm/Analysis/CaptureTracking.h:24
 
-  /// 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;
+  unsigned getDefaultMaxUsesToExploreForCaptureTracking();
 
----------------
skatkov wrote:
> jdoerfert wrote:
> > Documentation please.
> It is in cpp file. Would you like me to move it in hpp?
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. 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78624/new/

https://reviews.llvm.org/D78624





More information about the llvm-commits mailing list