<div dir="ltr">David, could you do this? I'm not sure where best to put it.</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jan 5, 2015 at 11:10 PM, Nick Lewycky <span dir="ltr"><<a href="mailto:nicholas@mxc.ca" target="_blank">nicholas@mxc.ca</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 01/05/2015 03:49 PM, Sean Silva wrote:<br>
</span><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
<br>
<br>
On Mon, Dec 29, 2014 at 11:23 PM, David Majnemer<br></span><div><div class="h5">
<<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a> <mailto:<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.<u></u>com</a>>> wrote:<br>
<br>
    ================<br>
    Comment at: include/llvm/CodeGen/<u></u>GCStrategy.h:71-73<br>
    @@ -69,1 +70,5 @@<br>
        protected:<br>
    +    bool UseStatepoints;       /// Uses gc.statepoints as opposed<br>
    to gc.roots,<br>
    +                               /// if set, none of the other<br>
    options can be<br>
    +                               /// anything but their default values.<br>
    +<br>
    ----------------<br>
    The other comments are formatted using ///<<br>
<br>
    Also, the layout of GCStrategy might be a little more compact if you<br>
    shifted this field down.<br>
<br>
    ================<br>
    Comment at: lib/CodeGen/SelectionDAG/<u></u>StatepointLowering.cpp:429<br>
    @@ +428,3 @@<br>
    +    GCStrategy &S = Builder.GFI->getStrategy();<br>
    +    for(const Value *V : Bases) {<br>
    +      auto Opt = S.isGCManagedPointer(V);<br>
    ----------------<br>
    There should be a space between the for and the (<br>
<br>
    ================<br>
    Comment at: lib/CodeGen/<u></u>StatepointExampleGC.cpp:29<br>
    @@ +28,3 @@<br>
    +  StatepointGC() {<br>
    +    UseStatepoints=true;<br>
    +    // These options are all gc.root specific, we specify them so<br>
    that the<br>
    ----------------<br>
    Space before and after =<br>
<br>
    ================<br>
    Comment at: lib/CodeGen/<u></u>StatepointExampleGC.cpp:39-40<br>
    @@ +38,4 @@<br>
    +  Optional<bool> isGCManagedPointer(const Value *V) const override {<br>
    +    PointerType *PT = dyn_cast<PointerType>(V-><u></u>getType());<br>
    +    assert(PT && "method is only valid on pointer typed values");<br>
    +    // For the sake of this example GC, we arbitrarily pick<br>
    addrspace(1) as our<br>
    ----------------<br>
    If this is the case, why not just cast<PointerType> ?<br>
<br>
    ================<br>
    Comment at: lib/CodeGen/<u></u>StatepointExampleGC.cpp:44<br>
    @@ +43,3 @@<br>
    +    // updated and that no other pointer does.<br>
    +    return (1 == PT->getAddressSpace());<br>
    +  }<br>
    ----------------<br>
    Address space 1 has a special meaning in LLVM, it's identical to<br>
    address space 0 except for the fact that "null" may be<br>
    dereferenced.  You might want to consider a different address space.<br>
<br>
<br>
Is this documented? I can't find anything in LangRef.<br>
</div></div></blockquote>
<br>
I don't think it ever made it down in writing. Please fix!<br>
<br>
Nick<br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
<br>
-- Sean Silva<br>
<br>
<br>
    <a href="http://reviews.llvm.org/D6808" target="_blank">http://reviews.llvm.org/D6808</a><br>
<br>
    EMAIL PREFERENCES<br>
    <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/<u></u>settings/panel/<u></u>emailpreferences/</a><br>
<br>
<br>
<br>
    ______________________________<u></u>_________________<br>
    llvm-commits mailing list<br></span>
    <a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a> <mailto:<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.<u></u>edu</a>><br>
    <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><span class=""><br>
<br>
<br>
<br>
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
<br>
</span></blockquote><div class="HOEnZb"><div class="h5">
<br>
______________________________<u></u>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/<u></u>mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>