[PATCH] Move ownership of GCStrategy objects to LLVMContext

[Ramkumar Ramachandra]

A small review, fwiw.


================
Comment at: include/llvm/CodeGen/GCMetadata.h:167
@@ -166,5 +166,3 @@
   class GCModuleInfo : public ImmutablePass {
-    typedef StringMap<GCStrategy*> strategy_map_type;
-    typedef std::vector<std::unique_ptr<GCStrategy>> list_type;
-
-    strategy_map_type StrategyMap;
+    typedef std::vector<GCStrategy*> list_type;
+    /// A list of GCStrategies which are active in this Module.  These are no
----------------
Why didn't you preserve std::unique_ptr?

================
Comment at: include/llvm/CodeGen/GCMetadata.h:170
@@ -169,3 +170,1 @@
-
-    strategy_map_type StrategyMap;
     list_type StrategyList;
----------------
You have a GCStrategyList, GCStrategyMap, and StrategyMap? Makes me wonder what happened to the corresponding StrategyList.

================
Comment at: include/llvm/CodeGen/GCStrategy.h:54
@@ -53,3 +53,3 @@
 #include "llvm/CodeGen/GCMetadata.h"
-#include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/Support/Registry.h"
----------------
This is unrelated to the overall change, isn't it? (Just asking)

================
Comment at: include/llvm/CodeGen/GCStrategy.h:70
@@ -66,3 +69,3 @@
     std::string Name;
-    friend class GCModuleInfo;
+    friend class LLVMContextImpl;
     
----------------
So this is the big change. GCModuleInfo benefited from this friendship by being able to instantiate and Name strategies from the GCRegistry. Now, you've removed that code and put it in ContextImpl.

================
Comment at: lib/CodeGen/GCMetadata.cpp:80
@@ -94,1 +79,3 @@
+  // Context. 
+  StrategyList.push_back(S);
   Functions.push_back(make_unique<GCFunctionInfo>(F, *S));
----------------
There is no user of this StrategyList. The only iterator I can see is in getGCStrategy(), and that uses GCStrategyList. What is this for?

================
Comment at: lib/IR/LLVMContextImpl.cpp:16
@@ -15,2 +15,3 @@
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/CodeGen/GCStrategy.h"
 #include "llvm/IR/Attributes.h"
----------------
Some final comments:
If I understand correctly, your patch helps us verify and optimize the code generated by gc plugins, hence making their lives easier.

http://reviews.llvm.org/D6811

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/