[WinEH] Add llvm.eh.exceptionpointer intrinsic

[JosephTremoulet]

Summary

This intrinsic can be used to extract a pointer to the exception caught by
a given catchpad. Its argument has token type and must be a catchpad.

Also clarify ExtendingLLVM documentation regarding overloaded intrinsics.

[rnk]

This seems like it's kind of splitting the difference between extracttoken and specific intrinsics. Based on your proposal, I was envisioning having a family of intrinsics like:

declare i32 @llvm.eh.exceptioncode(token)
declare i8* @llvm.eh.exceptioninfo(token) ; I guess this is for SEH filters only, which we don't have yet...
declare i8 @llvm.eh.abnormaltermination(token)
declare i32 @llvm.eh.exceptionstate(token)

Then we don't need the llvm_any_ty mangling, and the IR seems more readable.

What use cases do you have for the additional parameters to padparam? If the extra parameters are used like an enum, where the backend has to switch on it and use custom code for different cases, then I think individual intrinsics are the way to go. If the extra parameters are more like an index into a list of register parameters (RCX, RDX, R8, R9, ...) or an index into argument stack slots, then llvm.eh.padparam seems like a reasonable approach.

We should also see what Sanjoy needs for GC.

[JosephTremoulet]

Yes, it was ironic that eh.padparam ended up with the fully-general signature. That was simply an attempt to match the "look-and-feel" of the other EH operators/intrinsics, which are typically abstracted over the personality function.

What use cases do you have for the additional parameters to padparam?

I wasn't planning on using any in LLILC (the only extraction I need is to get an exception pointer from a catchpad), I just included them in the opcode signature to give flexibility for other personalities.

If the extra parameters are used like an enum, where the backend has to switch on it and use custom code for different cases, then I think individual intrinsics are the way to go. If the extra parameters are more like an index into a list of register parameters (RCX, RDX, R8, R9, ...) or an index into argument stack slots, then llvm.eh.padparam seems like a reasonable approach.

I agree with that view. I was imagining an outer switch on personality function, and that the selector arguments if used would likely index into a parameter list.

I'm certainly not wedded to keeping padparam this general. The specific signature that I need is exception_aggr addrspace(1)* @llvm.eh.intrinsicname() (or I could get by with i8 addrspace(1)* @llvm.eh.intrinsicname() and bitcasts). I guess for my specific case intrinsicname would be get_clr_exception or something. If you'd prefer that, let me know what you think the name should be and whether you want any of the ones you listed included in this patch, and I'll be happy to change it.

[rnk]

In D12533#237415, @JosephTremoulet wrote:

I'm certainly not wedded to keeping padparam this general. The specific signature that I need is exception_aggr addrspace(1)* @llvm.eh.intrinsicname() (or I could get by with i8 addrspace(1)* @llvm.eh.intrinsicname() and bitcasts). I guess for my specific case intrinsicname would be get_clr_exception or something. If you'd prefer that, let me know what you think the name should be and whether you want any of the ones you listed included in this patch, and I'll be happy to change it.

I guess the addressspace(1) thing has to be baked into the signature for you, so we can't get by with i8* @llvm.eh.exceptionpointer(token) and addresspacecast. How about making it llvm_any_ty and using i8 addressspace(1) * @llvm.eh.exceptionpointer.p1i8(token)?

[JosephTremoulet]

How about making it llvm_any_ty and using i8 addressspace(1) * @llvm.eh.exceptionpointer.p1i8(token)?

I can do something along those lines, sure. Two questions:

1. If it's always going to be a pointer type, would it be better to use llvm_anyptr_ty and i8 addrspace(1)* @llvm.eh.exceptionpointer.p1(token)?
2. OTOH, if it's not always going to be a pointer type (e.g. if you're going to use the same one for SEH exception codes), should the name be something like llvm.eh.get_exception that doesn't include pointer?

[rnk]

In D12533#237559, @JosephTremoulet wrote:

How about making it llvm_any_ty and using i8 addressspace(1) * @llvm.eh.exceptionpointer.p1i8(token)?

I can do something along those lines, sure. Two questions:

1. If it's always going to be a pointer type, would it be better to use llvm_anyptr_ty and i8 addrspace(1)* @llvm.eh.exceptionpointer.p1(token)?

Sounds good.

2. OTOH, if it's not always going to be a pointer type (e.g. if you're going to use the same one for SEH exception codes), should the name be something like llvm.eh.get_exception that doesn't include pointer?

If/when we add SEH filter funclets, we'll need separate constructs for exception_info() and exception_code(). In anticipation, we should probably use llvm.eh.exceptioncode for the SEH code.

[JosephTremoulet]

• Make the intrinsic and its signature more narrow, per review feedback

[JosephTremoulet]

I've made the updates per your suggestion (unless I've misunderstood the suggestion... :]).

[JosephTremoulet]

• fix typo in doc

[rnk]

lgtm

[sanjoy]

In D12533#237397, @rnk wrote:

We should also see what Sanjoy needs for GC.

We'd eventually (i.e. I cannot commit to a timeframe right now) change
statepoints to return a token, and change the existing gc_result
and gc_relocate family of intrinsics to consume the token produced
by the statepoint call / invoke. Currently we're using an i32
instead of a token, and that's unsound.

I expect the end result for the gc_relocate and gc_result
intrinsics to look fairly similar to llvm.eh.exceptionpointer.

docs/ExceptionHandling.rst

415

Should there be a verifier rule checking that only catchpads feed into this intrinsic?

[JosephTremoulet]

• add verifier rule and test

[JosephTremoulet]

docs/ExceptionHandling.rst

415

Yes; updated to include this.

[sanjoy]

In D12533#238308, @JosephTremoulet wrote:

• add verifier rule and test

LGTM.

docs/ExceptionHandling.rst

 of this intrinsic.  The intrinsic is provided for targets that require a more
 abstract interface.
 
 When used in the native Windows C++ exception handling implementation, this
 intrinsic serves as a placeholder to delimit code before a catch handler is
 outlined.  After the handler is outlined, this intrinsic is simply removed.
 
 
+.. _llvm.eh.exceptionpointer:
+
+``llvm.eh.exceptionpointer``
+----------------------
+
+.. code-block:: llvm
+
+  i8 addrspace(N)* @llvm.eh.padparam.pNi8(token %catchpad)
+
+
+This intrinsic retrieves a pointer to the exception caught by the given
+``catchpad``.
+
+
 SJLJ Intrinsics
 ---------------
 
 The ``llvm.eh.sjlj`` intrinsics are used internally within LLVM's
 backend.  Uses of them are generated by the backend's
 ``SjLjEHPrepare`` pass.
 
 .. _llvm.eh.sjlj.setjmp:

docs/ExtendingLLVM.rst

#. ``llvm/docs/LangRef.html``:
    Document the intrinsic.  Decide whether it is code generator specific and
    what the restrictions are.  Talk to other people about it so that you are
    sure it's a good idea.
 
 #. ``llvm/include/llvm/IR/Intrinsics*.td``:
 
    Add an entry for your intrinsic.  Describe its memory access characteristics
    for optimization (this controls whether it will be DCE'd, CSE'd, etc). Note
-   that any intrinsic using the ``llvm_int_ty`` type for an argument will
-   be deemed by ``tblgen`` as overloaded and the corresponding suffix will
-   be required on the intrinsic's name.
+   that any intrinsic using one of the ``llvm_any*_ty`` types for an argument or
+   return type will be deemed by ``tblgen`` as overloaded and the corresponding
+   suffix will be required on the intrinsic's name.
 
 #. ``llvm/lib/Analysis/ConstantFolding.cpp``:
 
    If it is possible to constant fold your intrinsic, add support to it in the
    ``canConstantFoldCallTo`` and ``ConstantFoldCall`` functions.
 
 #. ``llvm/test/*``:
 

include/llvm/IR/Intrinsics.td

 
 // eh.begincatch takes a pointer returned by a landingpad instruction and
 // copies the exception object into the memory pointed to by the second
 // parameter. If the second parameter is null, no copy occurs.
 def int_eh_begincatch : Intrinsic<[], [llvm_ptr_ty, llvm_ptr_ty],
                                   [NoCapture<0>, NoCapture<1>]>;
 def int_eh_endcatch : Intrinsic<[], []>;
 
+// eh.exceptionpointer returns the pointer to the exception caught by
+// the given `catchpad`.
+def int_eh_exceptionpointer : Intrinsic<[llvm_anyptr_ty], [llvm_token_ty],
+                                        [IntrNoMem]>;
+
 // Represents the list of actions to take when an exception is thrown.
 def int_eh_actions : Intrinsic<[llvm_ptr_ty], [llvm_vararg_ty], []>;
 
 def int_eh_exceptioncode : Intrinsic<[llvm_i32_ty], [], [IntrReadMem]>;
 
 // __builtin_unwind_init is an undocumented GCC intrinsic that causes all
 // callee-saved registers to be saved and restored (regardless of whether they
 // are used) in the calling function. It is used by libgcc_eh.

lib/IR/Verifier.cpp

case Intrinsic::experimental_gc_relocate: {
 
     // gc_relocate return type must be a pointer type, and is verified earlier in
     // VerifyIntrinsicType().
     Assert(cast<PointerType>(CS.getType())->getAddressSpace() ==
            cast<PointerType>(Operands.getDerivedPtr()->getType())->getAddressSpace(),
            "gc.relocate: relocating a pointer shouldn't change its address space", CS);
     break;
   }
+  case Intrinsic::eh_exceptionpointer: {
+    Assert(isa<CatchPadInst>(CS.getArgOperand(0)),
+           "eh.exceptionpointer argument must be a catchpad", CS);
+    break;
+  }
   };
 }
 
 /// \brief Carefully grab the subprogram from a local scope.
 ///
 /// This carefully grabs the subprogram from a local scope, avoiding the
 /// built-in assertions that would typically fire.
 static DISubprogram *getSubprogram(Metadata *LocalScope) {

test/CodeGen/WinEH/wineh-intrinsics-invalid.ll

+; RUN: sed -e s/.T1:// %s | not opt -lint -disable-output 2>&1 | FileCheck --check-prefix=CHECK1 %s
+; RUN: sed -e s/.T2:// %s | not opt -lint -disable-output 2>&1 | FileCheck --check-prefix=CHECK2 %s
+
+target triple = "x86_64-pc-windows-msvc"
+
+declare void @f()
+
+;T1: declare i8* @llvm.eh.exceptionpointer.p0i8(i32)
+;T1:
+;T1: define void @test1() personality i32 (...)* @__CxxFrameHandler3 {
+;T1:   call i8* @llvm.eh.exceptionpointer.p0i8(i32 0)
+;T1:   ret void
+;T1: }
+;CHECK1: Intrinsic has incorrect argument type!
+;CHECK1-NEXT: i8* (i32)* @llvm.eh.exceptionpointer.p0i8
+
+;T2: declare i8* @llvm.eh.exceptionpointer.p0i8(token)
+;T2:
+;T2: define void @test2() personality i32 (...)* @__CxxFrameHandler3 {
+;T2:   call i8* @llvm.eh.exceptionpointer.p0i8(token undef)
+;T2:   ret void
+;T2: }
+;CHECK2: eh.exceptionpointer argument must be a catchpad
+;CHECK2-NEXT:  call i8* @llvm.eh.exceptionpointer.p0i8(token undef)
+
+declare i32 @__CxxFrameHandler3(...)

test/CodeGen/WinEH/wineh-intrinsics.ll

+; RUN: opt -lint -disable-output < %s
+
+; This test is meant to prove that the verifier does not report errors for correct
+; use of the llvm.eh.exceptionpointer intrinsic.
+
+target triple = "x86_64-pc-windows-msvc"
+
+declare i8* @llvm.eh.exceptionpointer.p0i8(token)
+declare i8 addrspace(1)* @llvm.eh.exceptionpointer.p1i8(token)
+
+declare void @f(...)
+
+define void @test1() personality i32 (...)* @__CxxFrameHandler3 {
+entry:
+  invoke void (...) @f(i32 1)
+     to label %exit unwind label %catchpad
+catchpad:
+  %catch = catchpad [i32 1] to label %do_catch unwind label %catchend
+do_catch:
+  %exn = call i8* @llvm.eh.exceptionpointer.p0i8(token %catch)
+  call void (...) @f(i8* %exn)
+  catchret %catch to label %exit
+catchend:
+  catchendpad unwind to caller
+exit:
+  ret void
+}
+
+define void @test2() personality i32 (...)* @ProcessManagedException {
+entry:
+  invoke void (...) @f(i32 1)
+     to label %exit unwind label %catchpad
+catchpad:
+  %catch = catchpad [i32 1] to label %do_catch unwind label %catchend
+do_catch:
+  %exn = call i8 addrspace(1)* @llvm.eh.exceptionpointer.p1i8(token %catch)
+  call void (...) @f(i8 addrspace(1)* %exn)
+  catchret %catch to label %exit
+catchend:
+  catchendpad unwind to caller
+exit:
+  ret void
+}
+
+declare i32 @__CxxFrameHandler3(...)
+declare i32 @ProcessManagedException(...)