[PATCH] Introduce an example statepoint GC strategy

[Sean Silva]

David, could you do this? I'm not sure where best to put it.

On Mon, Jan 5, 2015 at 11:10 PM, Nick Lewycky <nicholas at mxc.ca> wrote:

> 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
>>
>>
> _______________________________________________
> 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/20150106/a3d0bd90/attachment.html>