<html>
<head>
<meta content="text/html; charset=windows-1252"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 05/02/2014 12:52 PM, Andrew Trick
wrote:<br>
</div>
<blockquote
cite="mid:A2AFFC0D-564B-4015-A7D0-B9106404603E@apple.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=windows-1252">
<br>
<div>
<div>On May 2, 2014, at 12:16 PM, Filip Pizlo <<a
moz-do-not-send="true" href="mailto:fpizlo@apple.com">fpizlo@apple.com</a>>
wrote:</div>
<br class="Apple-interchange-newline">
<blockquote type="cite">
<div style="font-family: Helvetica, Arial; font-size: 13px;
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;
word-wrap: break-word; -webkit-nbsp-mode: space;
-webkit-line-break: after-white-space;">
<div id="bloop_customfont" style="font-family: Helvetica,
Arial; font-size: 13px; margin: 0px;">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; margin: 0px;"><br>
</div>
<div id="bloop_customfont" style="font-family: Helvetica,
Arial; font-size: 13px; margin: 0px;">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>
<div id="bloop_customfont" style="font-family: Helvetica,
Arial; font-size: 13px; margin: 0px;"><br>
</div>
<div id="bloop_customfont" style="font-family: Helvetica,
Arial; font-size: 13px; margin: 0px;">Claiming in the
documentation that the point of a patchpoint is that it
always calls the call target would be confusing to anyone
wanting to implement inline caches using LLVM. The
purpose of inline caches is to change the call target, or
to eliminate the call altogether, using post-compilation
patch-up. The semantics of patchpoints should be
crystal-clear regarding this aspect of the intrinsic and
the fact that this is allowed.</div>
<div id="bloop_customfont" style="font-family: Helvetica,
Arial; font-size: 13px; margin: 0px;"><br>
</div>
<div id="bloop_customfont" style="font-family: Helvetica,
Arial; font-size: 13px; margin: 0px;">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>
<div id="bloop_customfont" style="font-family: Helvetica,
Arial; font-size: 13px; margin: 0px;"><br>
</div>
<div id="bloop_customfont" style="font-family: Helvetica,
Arial; font-size: 13px; margin: 0px;">There's also the
question of how not to break WebKit. 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>
<div id="bloop_customfont" style="font-family: Helvetica,
Arial; font-size: 13px; margin: 0px;"><br>
</div>
<div id="bloop_customfont" style="font-family: Helvetica,
Arial; font-size: 13px; margin: 0px;">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</div>
</div>
</blockquote>
<blockquote type="cite">
<div style="font-family: Helvetica, Arial; font-size: 13px;
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;
word-wrap: break-word; -webkit-nbsp-mode: space;
-webkit-line-break: after-white-space;">
<div id="bloop_customfont" style="font-family: Helvetica,
Arial; font-size: 13px; margin: 0px;"> talking about two
different semantics - one that means call and another that
means "clobber the world" - and so we should keep them
separate.</div>
</div>
</blockquote>
<div><br>
</div>
<div>If you pass a null target, then LLVM can’t assume anything.
So you will always have your “anything goes” semantics if you
want it.</div>
</div>
<div><br>
</div>
<div>Passing a symbolic target (which we don’t even support yet)
could convey to LLVM that the return value is consistent with
the call target. It doesn’t mean the target actually has to be
called. e.g. If I give patchpoint a @safe.div target with
operands a, b, I can expect the return value to be a/b. Some
other stuff could have happened too that doesn’t affect the
result.</div>
</blockquote>
This was exactly my original chain of reasoning. <br>
<blockquote
cite="mid:A2AFFC0D-564B-4015-A7D0-B9106404603E@apple.com"
type="cite">
<div><br>
</div>
<div>I do not like the idea of doing this for safe.div at all, but
we could certainly add more useful semantics to patchpoint
without affecting WebKit.</div>
<div><br>
</div>
<div>-Andy<br>
<blockquote type="cite">
<div style="font-family: Helvetica, Arial; font-size: 13px;
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;
word-wrap: break-word; -webkit-nbsp-mode: space;
-webkit-line-break: after-white-space;">
<p style="">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>On Fri, May
2, 2014 at 11:58 AM, Philip Reames <br>
<<a moz-do-not-send="true"
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
moz-do-not-send="true"
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 moz-do-not-send="true"
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</span></blockquote>
</div>
</blockquote>
</div>
<br>
</blockquote>
<br>
</body>
</html>