<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Oct 29, 2013 at 10:51 AM, Andrew Trick <span dir="ltr"><<a href="mailto:atrick@apple.com" target="_blank">atrick@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="im"><br>
On Oct 28, 2013, at 11:45 AM, Juergen Ributzka <<a href="mailto:juergen@apple.com">juergen@apple.com</a>> wrote:<br>
<br>
> Hi Chris,<br>
><br>
> 1.) The return type of the patchpoint intrinsic is independent of any of its arguments, so we couldn’t use the LLVMMatchType to let tblgen pattern match all the possible intrinsic names for us. A more generic approach would require extending tblgen to also handle this case.<br>
><br>
> 2.) Not sure what you mean, because both patchpoint intrinsic versions take llvm_ptr_ty as an argument (3rd argument).<br>
><br>
> Cheers,<br>
> Juergen<br>
><br>
> On Oct 26, 2013, at 6:04 PM, Chris Lattner <<a href="mailto:clattner@apple.com">clattner@apple.com</a>> wrote:<br>
><br>
>><br>
>> On Oct 21, 2013, at 8:11 PM, Andrew Trick <<a href="mailto:atrick@apple.com">atrick@apple.com</a>> wrote:<br>
>><br>
>>> Hi lhames, ributzka, echristo,<br>
>>><br>
>>> These new intrinsics require varargs support.<br>
>><br>
>> Why is the _void and _i64 embedded into the intrinsic name? For the first one, are you expecting other versions of this? On the second, can/should it just take a llvm_ptr_ty, so that it is properly generic over any pointer type?<br>
<br>
</div>Juergen’s answer above is correct. I can rephrase.<br>
<br>
llvm.experimental.safepoint only has/will have a single version of the intrinsic.<br>
<br>
llvm.experimental.patchpoint implies a safepoint but adds call-like semantics. Each return type that we support needs a different version of the intrinsic. WebKit only needs two: void and i64. I suggested i8* and double variants but they aren’t interested in using them yet. I’m happy to add speculative features, but my current policy is strictly as-needed.<br>
<br>
I don’t think there are any objections to the patch at this point. If anyone wants more time/discussion please let me know.<br>
<br></blockquote><div><br></div><div>I've largely been reviewing the documentation first, it's a lot of code review as a feature :)</div><div><br></div><div>I'm not objecting per-se, but I think there are some aspects of it I'm not a fan of and figured getting the documentation for the intrinsics nailed down first would be a good step.</div>
<div><br></div><div>-eric</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
-Andy<br>
<div class="HOEnZb"><div class="h5"><br>
>>><br>
>>> <a href="http://llvm-reviews.chandlerc.com/D1993" target="_blank">http://llvm-reviews.chandlerc.com/D1993</a><br>
>>><br>
>>> Files:<br>
>>> include/llvm/IR/Intrinsics.td<br>
>>><br>
>>> Index: include/llvm/IR/Intrinsics.td<br>
>>> ===================================================================<br>
>>> --- include/llvm/IR/Intrinsics.td<br>
>>> +++ include/llvm/IR/Intrinsics.td<br>
>>> @@ -455,6 +455,19 @@<br>
>>> llvm_ptr_ty],<br>
>>> [IntrReadWriteArgMem, NoCapture<2>]>;<br>
>>><br>
>>> +//===------------------------ Stackmap Intrinsics -------------------------===//<br>
>>> +//<br>
>>> +def int_experimental_stackmap : Intrinsic<[],<br>
>>> + [llvm_i32_ty, llvm_i32_ty, llvm_vararg_ty]>;<br>
>>> +def int_experimental_patchpoint_void : Intrinsic<[],<br>
>>> + [llvm_i32_ty, llvm_i32_ty,<br>
>>> + llvm_ptr_ty, llvm_i32_ty,<br>
>>> + llvm_vararg_ty]>;<br>
>>> +def int_experimental_patchpoint_i64 : Intrinsic<[llvm_i64_ty],<br>
>>> + [llvm_i32_ty, llvm_i32_ty,<br>
>>> + llvm_ptr_ty, llvm_i32_ty,<br>
>>> + llvm_vararg_ty]>;<br>
>>> +<br>
>>> //===-------------------------- Other Intrinsics --------------------------===//<br>
>>> //<br>
>>> def int_flt_rounds : Intrinsic<[llvm_i32_ty]>,<br>
>>> <D1993.1.patch>_______________________________________________<br>
>>> llvm-commits mailing list<br>
>>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div></div>