[PATCH] D14677: [LegacyPassManager] Reduce memory usage for AnalysisUsage

[Philip Reames via llvm-commits]

reames added inline comments.

================
Comment at: lib/IR/LegacyPassManager.cpp:582-587
@@ +581,8 @@
+    // of dependencies.
+    AnalysisUsage AU;
+    P->getAnalysisUsage(AU);
+
+    auto *Node = [&]() {
+      FoldingSetNodeID ID;
+      AUFoldingSetNode::Profile(ID, AU);
+      void *IP = nullptr;
----------------
chandlerc wrote:
> Since we build this completely each time, the folding set machinery isn't really hepling much. Maybe just provide a hash_value overload and use a SmallSet? Should be able to either delegate to the vector's hash_value or use hash_combine_range.
> 
> (This isn't because they're that much simpler, I'd just rather move everything away from folding sets where reasonable.)
I actually started with trying to use DenseSet, but the choice of empty and tombstone keys was non-obvious.  I'm not familiar enough with the AU data structure to identify values which could never occur.  

SmallSet is probably a bad choice.  I have no evidence the set of AU values is small, only that its elements are heavily reused.  Picking a good small size would be tricky.  In particular, the small size which works for O2 is probably not the one which works for my out of tree pipeline and vice versa.  

I would prefer to use the folding set here.  I believe it is the appropriate data structure choice and the usage pattern closely maps usages elsewhere (e.g. SCEV).  

================
Comment at: lib/IR/LegacyPassManager.cpp:594
@@ +593,3 @@
+      return Temp;
+    }();
+
----------------
chandlerc wrote:
> As sympathetic as I am to using multiple returns like this in an immediately called lamda, I find it quite confusing. Especially with the auto type on the variable.
> 
> Fortunately, I think with a set, the code to do this without a lambda shouldn't be onerous at all.
I slightly disagree, but minor.  Will fix in an update version shortly.  


http://reviews.llvm.org/D14677