[PATCH] D15244: [PassManager] Tuning Memory Usage of AnalysisUsage

[Chandler Carruth via llvm-commits]

chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.

LGTM

I agree that SmallVector seems ... dubious here. But it might not be worth it to try to change that. I like your change. I'd re-balance the numbers a bit differently based on my intuition (see the comment below) but I have no data. Also feel free to rebalance them based on actual data if you would like. Feel free to commit any updated numbers without pre-commit review also. =] Seems totally straightforward.


================
Comment at: include/llvm/IR/LegacyPassManagers.h:267-270
@@ -266,6 +266,6 @@
       ID.AddBoolean(AU.getPreservesAll());
-      for (auto &Vec : {AU.getRequiredSet(), AU.getRequiredTransitiveSet(),
-            AU.getPreservedSet(), AU.getUsedSet()}) {
-        ID.AddInteger(Vec.size());
-        for(AnalysisID AID : Vec)
+      for (auto *Vec : {&AU.getRequiredSet(), &AU.getRequiredTransitiveSet(),
+            &AU.getPreservedSet(), &AU.getUsedSet()}) {
+        ID.AddInteger(Vec->size());
+        for(AnalysisID AID : *Vec)
           ID.AddPointer(AID);
----------------
Alternatively, you could use:

  for (AnalysisUsage::VectorType &Vec : ...

================
Comment at: include/llvm/PassAnalysisSupport.h:43-46
@@ -42,3 +42,6 @@
   /// Sets of analyses required and preserved by a pass
-  VectorType Required, RequiredTransitive, Preserved, Used;
+  SmallVector<AnalysisID, 8> Required;
+  SmallVector<AnalysisID, 32> RequiredTransitive;
+  SmallVector<AnalysisID, 8> Preserved;
+  SmallVector<AnalysisID, 4> Used;
   bool PreservesAll;
----------------
FWIW, these don't match my basic expectations. I would expect Preserved to be the largest, followed by Required. They'll probably be close. Both RequiredTransitive and Used should be quite rare. I would go for 8, 8, 1, 1.


http://reviews.llvm.org/D15244