<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">Hi David and Joseph,</div><div class=""><br class=""></div><div class="">I’ve just added landingpad with token type locally and changed gc.relocate to work in the following way:</div><div class=""><br class=""></div><div class=""><div class=""> %0 = invoke token (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)</div><div class=""> to label %invoke_safepoint_normal_dest unwind label %exceptional_return</div><div class=""><br class=""></div><div class="">invoke_safepoint_normal_dest:</div><div class=""> ...</div><div class=""><br class=""></div><div class="">exceptional_return:</div><div class=""> %landing_pad = landingpad token</div><div class=""> cleanup</div><div class=""> %obj.relocated1 = call coldcc i64 addrspace(1)* @llvm.experimental.gc.relocate.p1i64(token %landing_pad, i32 13, i32 13)</div><div class=""> %obj1.relocated1 = call coldcc i64 addrspace(1)* @llvm.experimental.gc.relocate.p1i64(token %landing_pad, i32 14, i32 14)</div><div class=""> ret i64 addrspace(1)* %obj1.relocated1</div></div><div class=""><br class=""></div><div class=""><br class=""></div><div class="">Now gc.statepoint return a token type instead of i32 type, and gc.relocate also takes a token type as its first argument (the first argument should either be the corresponding gc.statepoint for call statepoint or invoke statepoint on the normal path, or a reference that could help find the corresponding gc.statepoint on the unwind the path). And since landingpad produces a token type here as well, it can be passed as the reference to the gc.relocate’s first argument. </div><div class=""><br class=""></div><div class="">To make this work, I have changed two parts of the code. First is how gc.relocate looks up for its corresponding gc.statepoint on the unwind path. It used to use the extracted selector value to find the landingpad and then use the landingpad to find the invoke instruction, which is the gc.statepoint. Now, it can use the landingpad directly to find the invoke instruction. The second part is to make landingpad work with token type. In LLVM’s front end (passes before SelectionDAG), there is no restrictions on what type a landingpad should have (there are test cases in LLVM that has landingpad of i8 or i32 type). However, in SelectionDAGBuilder::visitLandingPad, it is enforced that landingpad must be two-valued (type of { i8*, i32 }), in which way it can handle the exception pointer and selector value inside it. As the first step, I’d like to just add a check to see if the landingpad is of token type, and if so stop it and don’t bother to create the DAG nodes for the exception pointer and selector value (same as what happens during SjLj exceptions). This is enough to support the gc.statepoint work but will not support for C++ style exception handling with gc.statepoint. As for follow-up work, I’d like to add some support to extract selector value from token landingpad. I think we could either do it explicitly in IR (maybe add a intrinsic call extract.selector or something similar) or implicitly during SelectionDAG (in visitLandingPad, check if it’s token type, and if so add an additional transform to extract the exception pointer and selector value from the token). I dont have a concrete design right now and I am happy to take any other ideas. My plan is to get the first step checked in and incrementally work on the follow-up work. Does that sound a reasonable approach to you guys? </div><div class=""><br class=""></div><div class="">thanks,</div><div class="">chen </div><div class=""><br class=""></div><div class=""><br class=""></div><br class=""><div><blockquote type="cite" class=""><div class="">On Dec 2, 2015, at 9:47 AM, Chen Li <<a href="mailto:meloli87@gmail.com" class="">meloli87@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" class=""><div class=""><br class="Apple-interchange-newline">On Dec 1, 2015, at 11:14 PM, David Majnemer <<a href="mailto:david.majnemer@gmail.com" class="">david.majnemer@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div class="">While we support 'opaque' types nested within struct types, they are not exactly battle tested:</div><div class=""><br class=""></div><div class="">$ cat t.ll</div><div class=""><div class="">%opaque_ty = type opaque</div><div class=""><br class=""></div><div class="">%struct_ty = type { i32, %opaque_ty }</div><div class=""><br class=""></div><div class="">define %struct_ty @f(%struct_ty* %p) {</div><div class=""> %load = load %struct_ty, %struct_ty* %p</div><div class=""> ret %struct_ty %load</div><div class="">}</div></div><div class=""><br class=""></div><div class="">$ opt -O2 t.ll -S</div><div class="">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.<br class=""></div></div></div></blockquote><div class=""><br class=""></div>Thanks David! I’ve actually hacked to add token type into struct type and ended up with the same failure as above. I will take a look at the catchpad and cleanuppad code, and create a patch to add token landingpad and have you review it.</div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class=""></div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">thanks,</div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">chen</div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><br class=""></div><div style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""><blockquote type="cite" class=""><div class=""><div dir="ltr" class=""><div class=""><br class=""></div><div class="">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.</div><div class=""><br class=""></div><div class="">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.</div></div><div class="gmail_extra"><br class=""><div class="gmail_quote">On Tue, Dec 1, 2015 at 8:07 PM, Chen Li<span class="Apple-converted-space"> </span><span dir="ltr" class=""><<a href="mailto:meloli87@gmail.com" target="_blank" class="">meloli87@gmail.com</a>></span><span class="Apple-converted-space"> </span>wrote:<br class=""><blockquote class="gmail_quote" style="margin: 0px 0px 0px 0.8ex; border-left-width: 1px; border-left-color: rgb(204, 204, 204); border-left-style: solid; padding-left: 1ex;"><div class="HOEnZb"><div class="h5">Hi David,<br class=""><br class="">Sorry to bother you, but I would like to get some suggestions on your recent work of token type.<br class=""><br class="">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.<br class=""><br class="">Currently, gc.statepoint of InvokeInst works as below (the code snippet is from test/CodeGen/X86/statepoint-invoke.ll):<br class=""><br class=""> %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)<br class=""> to label %invoke_safepoint_normal_dest unwind label %exceptional_return<br class=""><br class="">invoke_safepoint_normal_dest:<br class=""> …<br class=""><br class="">exceptional_return:<br class=""> %landing_pad = landingpad { i8*, i32 }<br class=""> cleanup<br class=""> %relocate_token = extractvalue { i8*, i32 } %landing_pad, 1<br class=""> %obj.relocated1 = call coldcc i64 addrspace(1)* @llvm.experimental.gc.relocate.p1i64(i32 % relocate_token, i32 13, i32 13)<br class=""> %obj1.relocated1 = call coldcc i64 addrspace(1)* @llvm.experimental.gc.relocate.p1i64(i32 % relocate_token, i32 14, i32 14)<br class=""> ret i64 addrspace(1)* %obj1.relocated1<br class=""><br class=""><br class="">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.<br class=""><br class="">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.<br class=""><br class="">Does either of the approaches sound reasonable to you? Other ideas are also welcomed :)<br class=""><br class="">Thank you very much!<br class=""><br class="">Best,<br class="">Chen <span class="Apple-converted-space"> </span></div></div></blockquote></div></div></div></blockquote></div></div></blockquote></div><br class=""></body></html>