[PATCH] Move ownership of GCStrategy objects to LLVMContext

Nick Lewycky nicholas at mxc.ca
Mon Jan 12 23:18:13 PST 2015


Philip Reames wrote:

> Hi chandlerc, nicholas, sanjoy,

> 

> (I would appreciate a close review on this one.  I'm changing ownership in a non-trivial way and am introducing what might be considered a layering violation - IR owrning and returning pointers to a CodeGen class.)


Sorry but that's a non-starter right there. We can't have IR depending 
on Codegen (unless you want to make Codegen not depend on IR).

We can cheat a little; we could move GCStrategy.h into the IR and 
forward-declare MachineFunction and pass it by pointer to 
findCustomSafePoints.

Should there be a split, GCStrategy (IR) and MachineGCStrategy? There 
isn't enough IR-side user of the new API for me to judge how to deal 
with it, even http://reviews.llvm.org/D6808 pretty much just uses isGCManagedPointer.

+ for (GCRegistry::iterator I = GCRegistry::begin(),
+        E = GCRegistry::end(); I != E; ++I) {

Range-based for loop?

> Rather than have the GCStrategy object owned by the GCModuleInfo - which is an immutable analysis pass used mainly by gc.root - have it be owned by the LLVMContext.  This simplifies the ownership logic (i.e. can you have two instances of the same strategy at once?), but more importantly, allows us to access the GCStrategy in the middle end optimizer.  To this end, I add an accessor through Function which becomes the canonical way to get at a GCStrategy instance.

> 

> In the near future, this will allows me to move some of the checks from http://reviews.llvm.org/D6808 into the Verifier itself, and to introduce optimization legality predicates for some of the recent additions to InstCombine.  (These will follow as separate changes.)

> 

> http://reviews.llvm.org/D6811

> 

> Files:

> 

>   include/llvm/CodeGen/GCMetadata.h

>   include/llvm/CodeGen/GCStrategy.h

>   include/llvm/IR/Function.h

>   lib/CodeGen/GCMetadata.cpp

>   lib/IR/Function.cpp

>   lib/IR/LLVMContextImpl.cpp

>   lib/IR/LLVMContextImpl.h

> 

> EMAIL PREFERENCES

> 

>   http://reviews.llvm.org/settings/panel/emailpreferences/



http://reviews.llvm.org/D6811

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






More information about the llvm-commits mailing list