[PATCH] Move ownership of GCStrategy objects to LLVMContext

Philip Reames listmail at philipreames.com
Mon Jan 5 10:40:21 PST 2015


================
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
----------------
artagnon wrote:
> Why didn't you preserve std::unique_ptr?
The GCStrategy is no longer owned by the GCModuleInfo.

================
Comment at: include/llvm/CodeGen/GCMetadata.h:170
@@ -169,3 +170,1 @@
-
-    strategy_map_type StrategyMap;
     list_type StrategyList;
----------------
artagnon wrote:
> You have a GCStrategyList, GCStrategyMap, and StrategyMap? Makes me wonder what happened to the corresponding StrategyList.
Sorry, what?  I don't understand your comment.

================
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"
----------------
artagnon wrote:
> This is unrelated to the overall change, isn't it? (Just asking)
Not really.  I'm removing headers which were only required by the deleted code and adding back a header which was an indirect dependency.

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



================
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"
----------------
artagnon wrote:
> 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.
I'd frame this differently: "it helps us verify and optimize code compiled for targets with a garbage collector".  But the difference is minor.

http://reviews.llvm.org/D6811

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






More information about the llvm-commits mailing list