[llvm-commits] [patch] Fix the inlining of invokes into invokes for libUnwind-like EH implementations

[Bill Wendling]

On May 20, 2011, at 6:57 PM, John McCall wrote:

> I couldn't take it anymore, so I wrote up a dirty little patch to fix inlining through invokes.  The patch does two things:
> 
> 1) Calls (not invokes) of _Unwind_Resume in the inlined function are turned into branches to the unwind dest of the outer invoke.
> 
> This is required because we really want to be using _Unwind_Resume to resume after a cleanup, but libUnwind and the standard personality functions don't expect us to call _Unwind_Resume without handling the exception if the action table said we were going to handle it.  This is not the ideal solution to this problem — among other things, it requires every landing pad to be capable of correctly handling a cleanup action, even if only trivially — but it's an improvement over the current situation, which actually requires every landing pad to advertise itself as having a catch-all (and then actually handle that!).  The ideal solution is not really attainable while still gluing together EH CFGs.
> 
> 2) If a call to llvm.eh.selector can be found for the enclosing unwind destination, calls to llvm.eh.selector in the inlined function are augmented with the actions of the outer selector.
> 
> This allows the backend to assemble the correct set of unwind actions for a landing pad without gratuitous labor.
> 
> In no way is this patch a replacement for a proper revision of LLVM's EH IR:  the current IR is still awkward, challenging to optimize and lower to reasonable code, and almost inextricably tied to libUnwind-like unwinders and GCC-style personality functions.  But it eliminates the problems with spurious catch-alls that prevent the proper processing of uncaught exceptions, and it permits frontends to go back to emitting cleanups that call themselves cleanups  and  calls to _Unwind_Resume instead of _Unwind_Resume_or_Rethrow.
> 
Hi John,

This looks good, and it may supersede the slow code in DwarfEHPrepare.cpp that's trying to do similar things. A couple of things:

+static bool isUnwindResume(Value *value) {
  ...
+  if (fnType->isVarArg()) return false;

I think that the Ada front-end generates a call to _Unwind_Resume which takes more than one argument (and possibly a varargs).

+/// [LIBUNWIND] Check whether this selector is "only cleanups":
+///   call i32 @llvm.eh.selector(blah, blah, i32 0)
+static bool isCleanupOnlySelector(EHSelectorInst *selector) {
+  if (selector->getNumArgOperands() != 3) return false;
+  ConstantInt *val = dyn_cast<ConstantInt>(selector->getArgOperand(2));
+  return (val && val->isZero());
+}

This doesn't take 'filters' into account.

+      // Otherwise, we just append the outer selector to the inner selector.
+      SmallVector<Value*, 16> NewSelector;
+      for (unsigned i = 0, e = Inner->getNumArgOperands(); i != e; ++i)
+        NewSelector.push_back(Inner->getArgOperand(i));
+      for (unsigned i = 2, e = Outer->getNumArgOperands(); i != e; ++i)
+        NewSelector.push_back(Outer->getArgOperand(i));

This might not work 100% if there are filters and / or there is a catchall in one or both of the selectors. Basically, the llvm.eh.selector's structure will have to be interpreted for both selectors and reassembled.

-bw