[PATCH] D16342: Add a "gc-transition" operand bundle

Pat Gavlin via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 20 10:34:02 PST 2016


pgavlin added a comment.

LGTM aside from a few nits. Some suggested docs are above.


================
Comment at: docs/LangRef.rst:1609
@@ +1608,3 @@
+GC transition operand bundles are characterized by the
+``"gc-transition"`` operand bundle tag.
+
----------------
I'd suggest something like:

```
GC transition operand bundles are characterized by the
``"gc-transition"`` operand bundle tag. These operand
bundles mark a call as a transition between a function
with one GC strategy to a function with a different GC
strategy. If coordinating the transition between GC
strategies requires additional code generation at the
call site, these bundles may contain any values that
are needed by the generated code.
```

It may also be helpful to link to the "GC Transitions" section of the Statepoints doc.

================
Comment at: lib/IR/Verifier.cpp:2516
@@ -2515,2 +2515,3 @@
   // Verify that a callsite has at most one "deopt" and one "funclet" operand
   // bundle.
+  bool FoundDeoptBundle = false, FoundFuncletBundle = false,
----------------
Nit: this comment needs updating for GC transition bundles.

================
Comment at: lib/IR/Verifier.cpp:2526
@@ -2524,1 +2525,3 @@
     }
+    if (Tag == LLVMContext::OB_gc_transition) {
+      Assert(!FoundGCTransitionBundle, "Multiple gc-transition operand bundles",
----------------
Nit: this if statement and its successor could be `else if` clauses.


http://reviews.llvm.org/D16342





More information about the llvm-commits mailing list