<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Dec 29, 2014 at 11:23 PM, David Majnemer <span dir="ltr"><<a href="mailto:david.majnemer@gmail.com" target="_blank">david.majnemer@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">================<br>
Comment at: include/llvm/CodeGen/GCStrategy.h:71-73<br>
@@ -69,1 +70,5 @@<br>
   protected:<br>
+    bool UseStatepoints;       /// Uses gc.statepoints as opposed to gc.roots,<br>
+                               /// if set, none of the other 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 shifted this field down.<br>
<br>
================<br>
Comment at: lib/CodeGen/SelectionDAG/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/StatepointExampleGC.cpp:29<br>
@@ +28,3 @@<br>
+  StatepointGC() {<br>
+    UseStatepoints=true;<br>
+    // These options are all gc.root specific, we specify them so that the<br>
----------------<br>
Space before and after =<br>
<br>
================<br>
Comment at: lib/CodeGen/StatepointExampleGC.cpp:39-40<br>
@@ +38,4 @@<br>
+  Optional<bool> isGCManagedPointer(const Value *V) const override {<br>
+    PointerType *PT = dyn_cast<PointerType>(V->getType());<br>
+    assert(PT && "method is only valid on pointer typed values");<br>
+    // For the sake of this example GC, we arbitrarily pick addrspace(1) as our<br>
----------------<br>
If this is the case, why not just cast<PointerType> ?<br>
<br>
================<br>
Comment at: lib/CodeGen/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 address space 0 except for the fact that "null" may be dereferenced.  You might want to consider a different address space.<br></blockquote><div><br></div><div>Is this documented? I can't find anything in LangRef.</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><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/settings/panel/emailpreferences/</a><br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">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/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div></div>