[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