<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 05/02/2014 12:16 PM, Filip Pizlo
wrote:<br>
</div>
<blockquote cite="mid:etPan.5363eefd.26f324ba.172db@dethklok.local"
type="cite">
<style>body{font-family:Helvetica,Arial;font-size:13px}</style>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;">You're
proposing a unified intrinsic that allows Philip's call target
reasoning and the current "anything goes" semantics. I strongly
dislike unifying the two. I don't believe that the current
semantics are cleanly expressible using Philip's intrinsic.
Specifying patchpoint semantics in a way that implies that the
call target is always called breaks inline caches. Even if you
could make those things work, you'd still have to solve other
issues, like the types used to pass the call target.</div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;"><br>
</div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;">How would a
unified intrinsic work for dynamic object access inline caches
and virtual call inline caches? Presumably you'd want to have a
special fake call target that means "this can do anything". You
could say that null means "anything" but this feels ugly. Other
than that, my understanding is that LLVM IR currently doesn't
allow for any other kind of "this is a call target that is
guaranteed to not be analyzable" construct - hypothetically any
extern call could be resolved with LTO or something else.</div>
</blockquote>
Just to note, if we do settle on two different use cases - which I
think we will - I would oppose trying to unify them into a single
intrinsic. I think there's value in having them separate, even if
much of the actual implementation is shared.<br>
<br>
<br>
<blockquote cite="mid:etPan.5363eefd.26f324ba.172db@dethklok.local"
type="cite">
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;">Also, would
you expect LLVM phases that perform optimizations using the call
target to recover the call target by unwrapping the
function-pointer-to-i8* bitcast? This part is particularly
yucky to me.</div>
</blockquote>
Using an i8* is merely an implementation detail and could be
adjusted. For my local changes - not the ones under discussion! -
we added the function types as possible "any" types in the intrinsic
definition. This will hopefully be in shareable state in the next
week or so. <br>
<br>
<blockquote cite="mid:etPan.5363eefd.26f324ba.172db@dethklok.local"
type="cite">
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;"><br>
</div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;">There's also
the question of how not to break WebKit. <br>
</div>
</blockquote>
I want to explicitly disagree with this point. It is not the
community's responsibility to avoid breaking WebKit; it's yours. If
we find a "better" way of expressing the desired use cases
(including yours), we can and should change the current
implementation. The same holds true for my out of tree project and
everyone else's out there. I'm not saying we shouldn't consider
migration plans and "play nice" with existing implementations; I
would in fact argue for that quite strongly. <br>
<br>
<br>
<blockquote cite="mid:etPan.5363eefd.26f324ba.172db@dethklok.local"
type="cite">
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;">Note that you
can accomplish this by saying that a null call target means that
the call target can do anything, as I mention above - but I
don't like this because it seems like you're creating two
intrinsics and selecting them by passing null as one of the
arguments rather than selecting them by name.</div>
</blockquote>
I don't like this either. Overloading meaning is bad.<br>
<br>
Filip - Detail question. Are you using the null target to signal a
runtime trap? Or are you handling this specially during
installation? Just curious. <br>
<blockquote cite="mid:etPan.5363eefd.26f324ba.172db@dethklok.local"
type="cite">
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;"><br>
</div>
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;">Long story
short: creating unified intrinsics is useful if it clarifies
reasoning. In this case a single unified intrinsic doesn't
clarify anything. We're talking about two different semantics -
one that means call and another that means "clobber the world" -
and so we should keep them separate.</div>
</blockquote>
Agreed.<br>
<blockquote cite="mid:etPan.5363eefd.26f324ba.172db@dethklok.local"
type="cite">
<div id="bloop_customfont"
style="font-family:Helvetica,Arial;font-size:13px; color:
rgba(0,0,0,1.0); margin: 0px; line-height: auto;"><br>
</div>
<div id="bloop_sign_1399057275295571200" class="bloop_sign">
<div style="font-family:helvetica,arial;font-size:13px">-Phil</div>
<div style="font-family:helvetica,arial;font-size:13px"><br>
</div>
</div>
<br>
<p style="color:#000;">On May 2, 2014 at 12:00:40 PM, Eric
Christopher (<a moz-do-not-send="true"
href="mailto:echristo@gmail.com">echristo@gmail.com</a>)
wrote:</p>
<blockquote type="cite" class="clean_bq"><span>
<div>
<div>On Fri, May 2, 2014 at 11:58 AM, Philip Reames
<br>
<a class="moz-txt-link-rfc2396E" href="mailto:listmail@philipreames.com"><listmail@philipreames.com></a> wrote:
<br>
>
<br>
> On 05/02/2014 11:57 AM, Filip Pizlo wrote:
<br>
>
<br>
>
<br>
> On May 2, 2014 at 11:53:25 AM, Eric Christopher
(<a class="moz-txt-link-abbreviated" href="mailto:echristo@gmail.com">echristo@gmail.com</a>) wrote:
<br>
>
<br>
> On Wed, Apr 30, 2014 at 10:34 PM, Philip Reames
<br>
> <a class="moz-txt-link-rfc2396E" href="mailto:listmail@philipreames.com"><listmail@philipreames.com></a> wrote:
<br>
>> Andy - If you're not already following this
closely, please start. We've
<br>
>> gotten into fairly fundamental questions of what
a patchpoint does.
<br>
>>
<br>
>> Filip,
<br>
>>
<br>
>> I think you've hit the nail on the head. What I'm
thinking of as being
<br>
>> patchpoints are not what you think they are. Part
of that is that I've got
<br>
>> a local change which adds a very similar
construction (called
<br>
>> "statepoints"
<br>
>> for the moment), but I was trying to keep that
separate. That also
<br>
>> includes
<br>
>> a lot of GC semantics which are not under
discussion currently. My
<br>
>> apologies if that experience bled over into this
conversation and made
<br>
>> things more confusing.
<br>
>>
<br>
>> I will note that the documentation for patchpoint
say explicitly the
<br>
>> following:
<br>
>> "The ‘llvm.experimental.patchpoint.*‘ intrinsics
creates a function call
<br>
>> to
<br>
>> the specified <target> and records the
location of specified values in the
<br>
>> stack map."
<br>
>>
<br>
>> My reading has always been that a patchpoint
*that isn't patched* is
<br>
>> simply
<br>
>> a call with a stackmap associated with it. To my
reading, this can (and
<br>
>> did, and does) indicate my proposed usage would
be legal.
<br>
>>
<br>
>
<br>
> I like the idea that the target can be assumed to be
called. It makes
<br>
> optimization of the call possible, etc. I think it's
definitely worth
<br>
> exploring before we lock down the patchpoint
intrinsic.
<br>
>
<br>
> I will actively oppose this.
<br>
>
<br>
> I think it sounds like we need to split patchpoint
into two cases. I'm
<br>
> going to send a rough proposal for this later today.
<br>
>
<br>
<br>
A proposal definitely sounds interesting but if it makes
more sense to
<br>
keep it as single intrinsics then that's preferable. We've
already got
<br>
two in a kind of weird way with patchpoint and stackmap.
<br>
<br>
-eric
<br>
<br>
<br>
</div>
</div>
</span></blockquote>
</blockquote>
<br>
</body>
</html>