[llvm-dev] Support token type in struct for landingpad

[David Majnemer via llvm-dev]

While we support 'opaque' types nested within struct types, they are not
exactly battle tested:

$ cat t.ll
%opaque_ty = type opaque

%struct_ty = type { i32, %opaque_ty }

define %struct_ty @f(%struct_ty* %p) {
  %load = load %struct_ty, %struct_ty* %p
  ret %struct_ty %load
}

$ opt -O2 t.ll -S
lib/IR/DataLayout.cpp:623: unsigned int
llvm::DataLayout::getAlignment(llvm::Type *, bool) const: Assertion
`Ty->isSized() && "Cannot getTypeInfo() on a type that is unsized!"' failed.

As a practical matter, I fear nesting 'token' types within struct types
will have similar issues.  Beyond that, the design philosophy behind
'token' is that it is incredibly opaque and permitting it to nest inside a
struct creates scenarios where one might try to GEP to the end of the field
right before the token field in an attempt to examine or manipulate the
token.

Your other recommendation, having landingpad produce a token, is quite
similar to how we've designed catchpad and cleanuppad.  I think that
direction will be quite nice.

On Tue, Dec 1, 2015 at 8:07 PM, Chen Li <meloli87 at gmail.com> wrote:

> Hi David,
>
> Sorry to bother you, but I would like to get some suggestions on your
> recent work of token type.
>
> I’m currently working on changing gc.statepoint to return a token type
> instead of a i32 type. The reason is that with the current implementation,
> gc.statepoint could potentially be fed into PHI nodes, and break
> RewriteStatepointsForGC pass later. Using token type would help us to avoid
> this. I have most of the code work but got a problem when gc.statepint is
> an InvokeInst and has an unwind path.
>
> Currently, gc.statepoint of InvokeInst works as below (the code snippet is
> from test/CodeGen/X86/statepoint-invoke.ll):
>
>   %0 = invoke i32 (i64, i32, void (i64 addrspace(1)*)*, i32, i32, ...)
> @llvm.experimental.gc.statepoint.p0f_isVoidp1i64f(i64 0, i32 0, void (i64
> addrspace(1)*)* @some_call, i32 1, i32 0, i64 addrspace(1)* %obj, i32 0,
> i32 5, i32 0, i32 -1, i32 0, i32 0, i32 0, i64 addrspace(1)* %obj, i64
> addrspace(1)* %obj1)
>           to label %invoke_safepoint_normal_dest unwind label
> %exceptional_return
>
> invoke_safepoint_normal_dest:
>   …
>
> exceptional_return:
>   %landing_pad = landingpad { i8*, i32 }
>           cleanup
>   %relocate_token = extractvalue { i8*, i32 } %landing_pad, 1
>   %obj.relocated1 = call coldcc i64 addrspace(1)*
> @llvm.experimental.gc.relocate.p1i64(i32 % relocate_token, i32 13, i32 13)
>   %obj1.relocated1 = call coldcc i64 addrspace(1)*
> @llvm.experimental.gc.relocate.p1i64(i32 % relocate_token, i32 14, i32 14)
>   ret i64 addrspace(1)* %obj1.relocated1
>
>
> Each gc.relocate needs to take its corresponding gc.statepoint as its
> first argument. However, on the unwind path, there is no way to get
> gc.statepoint directly because the return value of the InvokeInst is
> undefined there. In this scenario, we tie gc.relocate to the landingpad,
> and use the landingpad to find its unique predecessor to get the
> corresponding gc.statepoint. We pick the selector value from the landingpad
> to feed into gc.relocate just because it has the same type (i32) as
> gc.statepoint's return type. The actual value of the selector doesn’t
> really matter because gc.relocate only uses it as a reference to find
> gc.statepoint and not consume it during lowering.
>
> However, this will no longer work if we change gc.statepoint's return type
> to token type. To make it work, I could see two potential approaches. 1)
> add support of token type inside struct type so that we can have a
> landingpad with result type of { i8*, token }, or 2) add support of
> landingpad with a token result type. Approach 1 seems to be easier since
> all the other parts of statepoint handling does not need to be changed at
> all, and having a selector of token type also seems reasonable
> (furthermore, we don’t ever need to extract selector value to do exception
> handling in our code base so I think only supporting token type in struct
> should be enough for us). Approach 2 requires to modify the way how
> gc.relocate looks up for its corresponding gc.statepoint through
> landingpad, but shouldn’t be hard either.
>
> Does either of the approaches sound reasonable to you? Other ideas are
> also welcomed :)
>
> Thank you very much!
>
> Best,
> Chen
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20151201/0ad9b43d/attachment.html>