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

[Joseph Tremoulet via llvm-commits]

JosephTremoulet added a comment.

LGTM aside from:

- two naming issues (which, since they're naming issues, are bikeshedding, and could always be changed post-commit anyway):
  - The term "outer scope" used for the "within" argument of pads seems discordant with the rest of our terminology.  I'd rather refer to them as "pads" since that's what we call them everywhere else, and just need an adjective.  Since the textual IR uses "within", having a good counterpart for that would work, something like `getOuterPad` or `getEnclosingPad` or `getContainingPad`.  Since in a few places we'll need to talk about the transitive closure of it, calling it `getParentPad` would also work for me, so that "ancestor" will be available for the transitive case.
  - I still think it's confusing that a `catchret`'s "outer" scope is the parent of the catchswitch.  I tried a few more suggestions inline, of course you'd want to s/Scope/Pad/ in them if the previous bullet sways you.
- one inline nit
- the thing Andy pointed out in SimplifyCFG


================
Comment at: include/llvm/IR/Instructions.h:4220
@@ +4219,3 @@
+  Value *getOuterScope() const {
+    return getCatchPad()->getCatchSwitch()->getOuterScope();
+  }
----------------
majnemer wrote:
> `getTargetScope` and `getDestinationScope` make you feel like they imply the existence of CFG edges which aren't really there.  I'll leave this alone unless a compelling name shows up.
I'd argue the edge is there -- you're getting the "scope" that contains the target of the CFG edge.  I think `getReturnedToScope` could make sense, but is awkward.  `getScopeAtDestination`?  `getOuterScopeOfCatchswitch`? `getNotExitedAncestorScope`? `getExitScope`?  `getInvokeScope`?  `getPreExceptionScope`? `getPreviousScope`? `getPriorScope`?

================
Comment at: include/llvm/IR/Instructions.h:3907
@@ -3903,3 +3906,3 @@
 
-  CatchPadInst(const CatchPadInst &CPI);
+  static BasicBlock *handler_helper(Value *V) { return cast<BasicBlock>(V); }
 
----------------
nit: seems like the two `handler_helper`s should be private.


http://reviews.llvm.org/D15139