[LLVMdev] Proposal: add intrinsics for safe division

[Philip Reames]

On 05/02/2014 12:16 PM, Filip Pizlo wrote:
> 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.
>
> 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.
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.


> 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.
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.

>
> There's also the question of how not to break WebKit.
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.


> 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.
I don't like this either.  Overloading meaning is bad.

Filip - Detail question.  Are you using the null target to signal a 
runtime trap?  Or are you handling this specially during installation?  
Just curious.
>
> 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.
Agreed.
>
> -Phil
>
>
> On May 2, 2014 at 12:00:40 PM, Eric Christopher (echristo at gmail.com 
> <mailto:echristo at gmail.com>) wrote:
>
>> On Fri, May 2, 2014 at 11:58 AM, Philip Reames
>> <listmail at philipreames.com> wrote:
>> >
>> > On 05/02/2014 11:57 AM, Filip Pizlo wrote:
>> >
>> >
>> > On May 2, 2014 at 11:53:25 AM, Eric Christopher 
>> (echristo at gmail.com) wrote:
>> >
>> > On Wed, Apr 30, 2014 at 10:34 PM, Philip Reames
>> > <listmail at philipreames.com> wrote:
>> >> Andy - If you're not already following this closely, please start. 
>> We've
>> >> gotten into fairly fundamental questions of what a patchpoint does.
>> >>
>> >> Filip,
>> >>
>> >> I think you've hit the nail on the head. What I'm thinking of as 
>> being
>> >> patchpoints are not what you think they are. Part of that is that 
>> I've got
>> >> a local change which adds a very similar construction (called
>> >> "statepoints"
>> >> for the moment), but I was trying to keep that separate. That also
>> >> includes
>> >> a lot of GC semantics which are not under discussion currently. My
>> >> apologies if that experience bled over into this conversation and 
>> made
>> >> things more confusing.
>> >>
>> >> I will note that the documentation for patchpoint say explicitly the
>> >> following:
>> >> "The ‘llvm.experimental.patchpoint.*‘ intrinsics creates a 
>> function call
>> >> to
>> >> the specified <target> and records the location of specified 
>> values in the
>> >> stack map."
>> >>
>> >> My reading has always been that a patchpoint *that isn't patched* is
>> >> simply
>> >> a call with a stackmap associated with it. To my reading, this can 
>> (and
>> >> did, and does) indicate my proposed usage would be legal.
>> >>
>> >
>> > I like the idea that the target can be assumed to be called. It makes
>> > optimization of the call possible, etc. I think it's definitely worth
>> > exploring before we lock down the patchpoint intrinsic.
>> >
>> > I will actively oppose this.
>> >
>> > I think it sounds like we need to split patchpoint into two cases. I'm
>> > going to send a rough proposal for this later today.
>> >
>>
>> A proposal definitely sounds interesting but if it makes more sense to
>> keep it as single intrinsics then that's preferable. We've already got
>> two in a kind of weird way with patchpoint and stackmap.
>>
>> -eric
>>
>>

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