llvm dev - Oct 2013 - [LLVMdev] [RFC] Stackmap and Patchpoint Intrinsic Proposal

On 10/23/13 5:38 PM, Andrew Trick wrote:
>
> On Oct 23, 2013, at 5:27 PM, Philip Reames <listmail at philipreames.com
> <mailto:listmail at philipreames.com>> wrote:
>
>>> 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.
>
> I want this to be clear when we expose the intrinsics to other 
> developers. I’ve been making an effort to improve the docs: 
> http://llvm-reviews.chandlerc.com/D1981. Please let me know where 
> clarification is needed
I responded in the review.  The only big thing that might be worth 
discussion here is the "full resume" semantics which are mentioned at 
the very end.  This seemed to disagree with our previous discussion.  
Let me know if you're either a) unclear at what I was getting at or b) 
believe the "full resume" semantics were a key part of the intrinsic.
In either case, we should probably hash it out here.

Philip

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131023/f057327a/attachment.html>

On Oct 23, 2013, at 7:26 PM, Philip Reames <listmail at philipreames.com>
wrote:
> On 10/23/13 5:38 PM, Andrew Trick wrote:
>> 
>> On Oct 23, 2013, at 5:27 PM, Philip Reames <listmail at
philipreames.com> wrote:
>> 
>>>> 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.
>> 
>> 
>> I want this to be clear when we expose the intrinsics to other
developers. I’ve been making an effort to improve the docs:
http://llvm-reviews.chandlerc.com/D1981. Please let me know where clarification
is needed
> I responded in the review.  The only big thing that might be worth
discussion here is the "full resume" semantics which are mentioned at
the very end.  This seemed to disagree with our previous discussion.  Let me
know if you're either a) unclear at what I was getting at or b) believe the
"full resume" semantics were a key part of the intrinsic.  In either
case, we should probably hash it out here.
Thanks for the review. I'll respond to each of your comments there.

There's one important thing I'm not getting across. llvm.stackmap is not
primarily intended for patching. As an additional feature, llvm.stackmap can
specify the size of its shadow, but does not need to generate nops in its
shadow. The shadow can contain arbitrary code from the same function. The only
purpose of the shadow is to allow destructive patching. Destructive means that
other that some arbitrary code may be overwritten. I think the only use for this
is to invalidate compiled code in a way that allows the runtime to recover. That
just happens to be a very important use. Note that if we didn't specify a
shadow, the runtime would have a hard time knowing how to patch without
potentially clobbering something outside the current function.

-Andy
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131023/d7706aa3/attachment.html>

On 10/23/13 10:03 PM, Andrew Trick wrote:
>
> On Oct 23, 2013, at 7:26 PM, Philip Reames <listmail at philipreames.com
> <mailto:listmail at philipreames.com>> wrote:
>
>> On 10/23/13 5:38 PM, Andrew Trick wrote:
>>>
>>> On Oct 23, 2013, at 5:27 PM, Philip Reames 
>>> <listmail at philipreames.com <mailto:listmail at
philipreames.com>> wrote:
>>>
>>>>> 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.
>>>
>>> I want this to be clear when we expose the intrinsics to other 
>>> developers. I’ve been making an effort to improve the docs: 
>>> http://llvm-reviews.chandlerc.com/D1981. Please let me know where 
>>> clarification is needed
>> I responded in the review.  The only big thing that might be worth 
>> discussion here is the "full resume" semantics which are
mentioned at
>> the very end.  This seemed to disagree with our previous discussion.  
>> Let me know if you're either a) unclear at what I was getting at or
>> b) believe the "full resume" semantics were a key part of the
>> intrinsic.  In either case, we should probably hash it out here.
>
> Thanks for the review. I'll respond to each of your comments there.
>
> There's one important thing I'm not getting across. llvm.stackmap
is
> not primarily intended for patching. As an additional 
> feature, llvm.stackmap can specify the size of its shadow, but does 
> not need to generate nops in its shadow. The shadow can contain 
> arbitrary code from the same function. The only purpose of the shadow 
> is to allow destructive patching. Destructive means that other that 
> some arbitrary code may be overwritten. I think the only use for this 
> is to invalidate compiled code in a way that allows the runtime to 
> recover. That just happens to be a very important use. Note that if we 
> didn't specify a shadow, the runtime would have a hard time knowing 
> how to patch without potentially clobbering something outside the 
> current function.
So essentially you're using the shadow region to enable creating either 
a nop sled which carries you down into deopt code or a forced trap (i.e. 
patching with an instruction sequence which forces a trap to a signal 
handler)?  Ok, that sounds reasonable.  You should spell this out this 
example explicitly in the documentation.  It's useful for clarification 
purposes.

Yours,
Philip



-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131024/187b2347/attachment.html>