[PATCH] Introduce an example statepoint GC strategy

[Sean Silva]

On Mon, Dec 29, 2014 at 11:23 PM, David Majnemer <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.

-- 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
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150105/3a37a2b8/attachment.html>