[PATCH] Introduce an example statepoint GC strategy

[Nick Lewycky]

On 01/05/2015 03:49 PM, Sean Silva wrote:
>
>
> On Mon, Dec 29, 2014 at 11:23 PM, David Majnemer
> <david.majnemer at gmail.com <mailto:david.majnemer at gmail.com>> wrote:
>
>     ================
>     Comment at: include/llvm/CodeGen/GCStrategy.h:71-73
>     @@ -69,1 +70,5 @@
>         protected:
>     +    bool UseStatepoints;       /// Uses gc.statepoints as opposed
>     to gc.roots,
>     +                               /// if set, none of the other
>     options can be
>     +                               /// anything but their default values.
>     +
>     ----------------
>     The other comments are formatted using ///<
>
>     Also, the layout of GCStrategy might be a little more compact if you
>     shifted this field down.
>
>     ================
>     Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:429
>     @@ +428,3 @@
>     +    GCStrategy &S = Builder.GFI->getStrategy();
>     +    for(const Value *V : Bases) {
>     +      auto Opt = S.isGCManagedPointer(V);
>     ----------------
>     There should be a space between the for and the (
>
>     ================
>     Comment at: lib/CodeGen/StatepointExampleGC.cpp:29
>     @@ +28,3 @@
>     +  StatepointGC() {
>     +    UseStatepoints=true;
>     +    // These options are all gc.root specific, we specify them so
>     that the
>     ----------------
>     Space before and after =
>
>     ================
>     Comment at: lib/CodeGen/StatepointExampleGC.cpp:39-40
>     @@ +38,4 @@
>     +  Optional<bool> isGCManagedPointer(const Value *V) const override {
>     +    PointerType *PT = dyn_cast<PointerType>(V->getType());
>     +    assert(PT && "method is only valid on pointer typed values");
>     +    // For the sake of this example GC, we arbitrarily pick
>     addrspace(1) as our
>     ----------------
>     If this is the case, why not just cast<PointerType> ?
>
>     ================
>     Comment at: lib/CodeGen/StatepointExampleGC.cpp:44
>     @@ +43,3 @@
>     +    // updated and that no other pointer does.
>     +    return (1 == PT->getAddressSpace());
>     +  }
>     ----------------
>     Address space 1 has a special meaning in LLVM, it's identical to
>     address space 0 except for the fact that "null" may be
>     dereferenced.  You might want to consider a different address space.
>
>
> Is this documented? I can't find anything in LangRef.

I don't think it ever made it down in writing. Please fix!

Nick

>
> -- Sean Silva
>
>
>     http://reviews.llvm.org/D6808
>
>     EMAIL PREFERENCES
>     http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
>     _______________________________________________
>     llvm-commits mailing list
>     llvm-commits at cs.uiuc.edu <mailto:llvm-commits at cs.uiuc.edu>
>     http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>