[PATCH] D15139: [IR] Reformulate LLVM's EH funclet IR

[Andy Kaylor via llvm-commits]

andrew.w.kaylor added inline comments.

================
Comment at: docs/ExceptionHandling.rst:632
@@ -635,2 +631,3 @@
 the function control is returned to.  A cleanup handler which reaches its end
 by normal execution executes a ``cleanupret`` instruction, which is a terminator
+indicating where the active exception will unwind to next.
----------------
How would you feel about renaming cleanupret to something like cleanupcontinue?  The current naming gives the impression that catchret and cleanupret will do more or less the same thing, whereas they actually do fairly different things.  That is, in my mental categorization, catchret returns out of the exception handling process while cleanupret continues to the next step in the exception handling process.

================
Comment at: docs/ExceptionHandling.rst:712
@@ +711,3 @@
+
+Funclet parent tokens
+-----------------------
----------------
It would be helpful if the example above were updated to show what this looks like.

================
Comment at: docs/ExceptionHandling.rst:724
@@ +723,3 @@
+The ``catchswitch`` instruction does not create a funclet, but it produces a
+token that is always consumed by its immediate successor ``catchpad``
+instructions. This ensures that every catch handler modelled by a ``catchpad``
----------------
It would also be helpful if the example showed more than one catchpad in the same catchswitch and chained catchswitch instructions.  Something like this:
```
    try {
      try {
        Cleanup obj;
        may_throw();
      } catch (int e) {
        may_throw();
        return e;
      } catch (float f) {
        return -1;
      }
    } catch (...) {
      // fall through
    }
```

================
Comment at: docs/LangRef.rst:5328
@@ -5331,1 +5327,3 @@
+      <resultval> = catchswitch <parent>, unwind to caller [<handlers>*]
+      <resultval> = catchswitch <parent>, unwind label %<default> [<handlers>*]
 
----------------
The <handlers> argument should be documented as "[ label <handler1>, label <handler2>, ... ]" to match similar instructions like indirectbr.

================
Comment at: docs/LangRef.rst:5334
@@ +5333,3 @@
+The '``catchswitch``' instruction is used by `LLVM's exception handling system
+<ExceptionHandling.html#overview>`_ describe the set of possible catch handlers
+that may be executed by the :ref:`EH personality routine <personalityfn>`.
----------------
describe -> to describe

================
Comment at: docs/LangRef.rst:5342
@@ -5353,1 +5341,3 @@
+``catchswitch`` instruction. If the ``catchswitch`` is not inside a funclet,
+this operand may be the token ``none``.
 
----------------
I feel like this syntax doesn't read well.  Since having 'none' as the parent will be the majority case, it feels clunky to have "catchswitch none" all over the place, and it isn't very intuitive what the 'none' token means.

What would you think of this?
```
<resultval> = catchswitch [from <parent>] unwind to caller [<handlers>*]
<resultval> = catchswitch [from <parent>] label %<default> [<handlers>*]
```
This way if there is no parent that part of the statement can just be omitted.

================
Comment at: docs/LangRef.rst:5389
@@ +5388,3 @@
+system <ExceptionHandling.html#overview>`_ to specify that a basic block
+is a catch handler --- one where a personality routine attempts to transfer
+control to catch an exception.
----------------
is -> begins ?

================
Comment at: docs/LangRef.rst:5402
@@ +5401,3 @@
+requires to know if this is an appropriate handler for the exception. Control
+will transfer to the ``catchpad`` if this is the case, or another successor of
+the preceding ``catchswitch`` if not.
----------------
"or another...."

This isn't strictly true, since it may also unwind to caller.


http://reviews.llvm.org/D15139