[PATCH] Make llvm.eh.begincatch use an outparam

Reid Kleckner rnk at google.com
Fri Feb 27 11:11:19 PST 2015


On Fri, Feb 27, 2015 at 10:46 AM, Andy Kaylor <andrew.kaylor at intel.com>
wrote:

> I don't think this is necessary.
>
> I've got an extra level of indirection that shouldn't be there in my
> current handling of the exception object, but I think the basic idea is
> sound.  My intention is to assign a fixed spot (element 1) in the frame
> allocation block to the exception object and then bitcast it to the correct
> type in the handler.  The llvm.eh.begin catch call tells me where the
> handler code wants to access the exception object and the bitcasts are
> already being generated.
>
> So like I said, what the currently committed code is doing is wrong, but
> it's pretty close.  Here's what we are currently generating for the catch
> handler in cppeh-catch-scalar:
>
> define i8* @_Z4testv.catch(i8*, i8*) {
> catch.entry:
>
>   %eh.alloc = call i8* @llvm.framerecover(i8* bitcast (void ()* @_Z4testv
> to i8*), i8* %1)
>   %eh.data = bitcast i8* %eh.alloc to %struct._Z4testv.ehdata*
>   %eh.obj.ptr = getelementptr inbounds %struct._Z4testv.ehdata* %eh.data,
> i32 0, i32 1
>   %eh.obj = load i8** %eh.obj.ptr
>   %i = getelementptr inbounds %struct._Z4testv.ehdata* %eh.data, i32 0,
> i32 2
>   %2 = bitcast i8* %eh.obj to i32*
>   %3 = load i32* %2, align 4
>   store i32 %3, i32* %i, align 4
>   %4 = load i32* %i, align 4
>   call void @_Z10handle_inti(i32 %4)
>   ret i8* blockaddress(@_Z4testv, %try.cont)
>
> }
>
> The GEP for %i is an unnecessary artifact as %i is not (and cannot be)
> referenced outside of the catch handler.  Also, there is a load-store-load
> sequence that confuses things but is just an artifact of having started
> with unoptimized IR.  The big issue, however, is that the indirection with
> %eh.obj.ptr shouldn't be there.  I didn't have that originally and then I
> guess one day I wasn't thinking clearly and convinced myself that I needed
> it.  Without that, I think you can see how this ought to work with the
> existing form of llvm.eh.begincatch.
>
> Here is what I think we should be generating:
>
> define i8* @_Z4testv.catch(i8*, i8*) {
> catch.entry:
>
>   %eh.alloc = call i8* @llvm.framerecover(i8* bitcast (void ()* @_Z4testv
> to i8*), i8* %1)
>   %eh.data = bitcast i8* %eh.alloc to %struct._Z4testv.ehdata*
>   ; "%4 = call i8* @llvm.eh.begincatch(i8* %exn11)" gets mapped to the
> instruction below
>   %eh.obj = getelementptr inbounds %struct._Z4testv.ehdata* %eh.data, i32
> 0, i32 1
>   %2 = bitcast i8* %eh.obj to i32*
>   %i = load i32* %2, align 4
>   call void @_Z10handle_inti(i32 %i)
>   ret i8* blockaddress(@_Z4testv, %try.cont)
>
> }
>
> I need to do some experimentation to see what happens when the exception
> object is bigger than a pointer size, but I'm fairly certain that this
> model can handle that just by putting some padding in the frame allocation
> structure.
>

Right, this change came out of me trying to come up with a solution to that
problem.

In Clang's current IR, what information would you use to size the exception
object slot? Currently it looks a bit like this:

; 'catch (A e)' where A is an aggregate.
%e.addr = alloca %struct.A
...
%ehobj = call i8* @llvm.eh.begincatch(i8* %exn)
%ehobj.A = bitcast i8* %ehobj to %struct.A*
call void @A_copy_ctor(%struct.A* %e.addr, %struct.A* %ehobj.A)
... ; catch body
call void @A_dtor(%struct.A* %e.addr)

LLVM doesn't know anything about C++ copy constructors, and it shouldn't.
Furthermore, this copy constructor call is completely missing from MSVC's
catch block code. The thrower is actually responsible for emitting "throw
info" describing all the ways you could catch the exception and how to copy
it if it is caught. Consider:

struct A { int a; A(); A(const A &); ~A();};
struct B : A { int b; B(); B(const B &); ~B();};
extern "C" void use_e(A *);
extern "C" void f() {
  try {
    throw B();
  } catch (A e) {
    use_e(&e);
  }
}

The only reference to A::A(const A &) is from a "throw information" table
that David is currently looking at. There are no A() constructor calls in
'f' or it's subfunctions, just a call to ~A() to cleanup e.

What I'm getting at is that the *runtime* is responsible for storing things
into the EH object stack slot. I think the best way to represent that in
the IR is to escape a stack object to an intrinsic. That tells the
optimizer that "stores can happen here", and that subsequent reads will be
properly initialized.

Note also that I am intentionally using the same stack location for the
> exception object in all catch handlers.  I can't see any advantage to not
> doing so.


What about this case?

void f() {
  try {
    might_throw();
  } catch (int e1) {
    try {
      might_throw2();
    } catch (int e2) {
      use_both_exceptions(e1, e2);
    }
  }
}

I think we need distinct storage for e1 and e2. The __CxxFrameHandler3
tables appear to support this, since each catch handler entry has space for
a distinct frame offset.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150227/f562fe3e/attachment.html>


More information about the llvm-commits mailing list