[LLVMdev] [RFC] Stackmap and Patchpoint Intrinsic Proposal

[Philip Reames]

On 10/22/13 10:48 PM, Andrew Trick wrote:
> I'll respond to a few questions below. I'll start a new thread for GC 
> discussion.
Good idea.  Thanks.
>
> On Oct 22, 2013, at 6:24 PM, Philip R <listmail at philipreames.com 
> <mailto:listmail at philipreames.com>> wrote:
>
>>> Now with regard to patching. I think llvm.patchpoint is generally 
>>> useful for any type of patching I can imagine. It does look like a 
>>> call site in IR, and it’s nice to be able to leverage calling 
>>> conventions to inform the location of arguments.
>> Agreed.  My concern is mostly about naming and documentation of 
>> intended usages. Speaking as someone who's likely to be using this in 
>> the very near future, I'd like to make sure I understand how you 
>> intend it to be used.  The last thing I want to do is misconstrue 
>> your intent and become reliant on a quirk of the implementation you 
>> later want to change.
>
> I don't think the intrinsic names will be able to capture their 
> semantics. I think that's why we need documentation, which I've been 
> working on: http://llvm-reviews.chandlerc.com/D1981.
>
> For example, the "stackmap" intrinsic isn't really a stack map, it's 
> something that allows generation of a stack map in which the entries 
> don't actually need to be on the stack... confusing, but still a good 
> name I think.
"stack map" is also a fairly well understood term in the GC/compiler 
world.  It's better to stick with well known terminology where possible.

As for naming vs documentation, I tend to believe that naming should be 
as descriptive as is reasonable.  Having said that, we're well past the 
point of adding value with this discussion.  Please update the 
documentation to reflect some of the clarifications on usage that have 
come up here and let's move on.
>
>>> But the patchpoint does not have to be a call after patching, and 
>>> you can specify zero arguments to avoid using a calling convention.
>> Er, not quite true.  Your calling convention also influences what 
>> registers stay live across the call.  But in general, I see your point.
>
> You get around that by defining a new calling convention. Each 
> patchpoint intrinsic call can be marked with a different calling 
> convention if you choose. For example, we'll be adding a dynamic 
> calling convention called AnyRegCC. You can use that to effectively 
> specify the number of arguments that you want to force into registers. 
> The stack map will tell you which registers were used for arguments. 
> The "call" will preserves most registers, but clobbers one register 
> (on x86) for use within the code.
Nice trick.  I'll have to remember that.
>
> Another potential extension is to add an entry to the stackmap marking 
> physical registers that are actually in-use across the stack map or 
> patch point.
>
> It helps me to think of llvm.patchpoint as a replacement for any 
> situation where a JIT would have otherwise needed inline asm to 
> generate the desired code sequence.
>
>> (Again, this is touching an area of LLVM I'm not particularly 
>> familiar with.)
>>> In fact, we only currently emit a call out of convenience. We could 
>>> splat nops in place and assume the runtime will immediately find and 
>>> patch all occurrences before the code executes. In the future we may 
>>> want to handle NULL call target, bypass call emission, and allow the 
>>> reserved bytes to be less than that required to emit a call.
>> If you were to do that, how would the implementation be different 
>> then the new stackmap intrinsic?  Does that difference imply a 
>> clarification in intended usage or naming?
>
> The implementation of the two intrinsics is actually very similar. In 
> this case, the difference would be that llvm.stackmap does not reserve 
> space for patching, while llvm.patchpoint does.
I'm slightly confused by this given that stackmap takes an argument 
indicating the number of nops to emit as well, but it's not worth 
debating this any more.  Let's move on.  We can revisit this once I'm 
actually using the new intrinsics and can provide real concrete feedback.
>
> We could have defined different intrinsics for all variations of use 
> cases, but I think two is the right number:
>
> - Use llvm.stackmap if you just want a stack map. No code will be 
> emitted. There is no calling convention. If the runtime patches the 
> code here, it will be destructive.
>
> - Use llvm.patchpoint if you want to reserve space for patching the 
> code. When you do that, you can optionally specify a number of 
> arguments that will follow a specified calling convention. You also 
> get a stack map here because it can be useful to fuse the stack map to 
> the point point location. After all, the runtime needs to know where 
> to patch.
This summary is really helpful.

This summary and the other points you've made in our discussion is 
exactly what should be in the documentation.  It provides the thought 
process behind their design, and the intended usage scenarios.  Can you 
add a section with this meta information?

Yours,
Philip

p.s. Thank you both for taking the time to hash this through.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20131023/ef8a1ed4/attachment.html>