[PATCH] "speculatable" function attribute
Chris Lattner
clattner at apple.com
Mon Jul 29 22:08:56 PDT 2013
On Jul 28, 2013, at 3:26 AM, "Kuperstein, Michael M" <michael.m.kuperstein at intel.com> wrote:
> (Moving this to the mailing list where it belongs)
>
> Fixed CallInst, merged with Tobias' patch, and added a description to LangRef.
> Can someone check the LangRef description makes sense to anyone but me?
>
> Michael
Hi Michael,
This is big and important enough to be worth a thread on llvmdev. Can you start a new discussion there along with the motivation from the beginning?
-Chris
>
> -----Original Message-----
> From: Nick Lewycky [mailto:nicholas at mxc.ca]
> Sent: Thursday, July 25, 2013 12:11
> To: Kuperstein, Michael M
> Cc: Nick Lewycky; llvmdev at cs.uiuc.edu; Tobias Grosser; echristo at gmail.com
> Subject: Re: [LLVMdev] Does nounwind have semantics?
>
> Kuperstein, Michael M wrote:
>> A patch is attached.
>
> + const CallInst* CI = dyn_cast<CallInst>(Inst);
> + return CI->isSafeToSpeculativelyExecute();
>
> "return cast<CallInst>(Inst)->isSafeToSpeculativelyExecute();"?
>
> Use cast<> instead of dyn_cast<>. See
> http://llvm.org/docs/ProgrammersManual.html#isa . Then I don't think it needs to be two lines. You can even remove the extra curly braces around this case.
>
>> Not sure I'm happy with this due to the aforementioned orthogonality
>> concerns, but I really don't have any better ideas. If anyone does,
>> feel free to offer them, I don't mind throwing this patch into the trash.
>>
>> (Also, not happy with the name, using "speculatable" as Nick
>> suggested, for the lack of other options. If the name stays I'll add
>> it to the
>> documentation.)
>
> That reminds me, the patch needs to come with an update to LangRef.rst.
>
>> Regarding auditing the intrinsics - I'd prefer to do this in stages.
>
> Sounds fine. I'd like Eric to take a quick look and agree that marking debug intrinsics speculatable is sane. (Yes they already are, but that doesn't mean it's sane. I also want Eric to know that 'speculatable' is going to start showing up in his debug info.)
>
> One other thing, your patch and Tobias Grosser's patch (subject "Make .bc en/decoding of AttrKind stable") are in conflict. Whoever lands second will need to merge.
>
> Nick
>
>> Here I'm just preserving the current behavior by marking intrinsics
>> that used to be explicitly handled in isSafeToSpeculativelyExecute(),
>> so there should be no functional change.
>>
>> *From:*Nick Lewycky [mailto:nlewycky at google.com]
>> *Sent:* Tuesday, July 23, 2013 02:29
>> *To:* Kuperstein, Michael M
>> *Cc:* Nick Lewycky; llvmdev at cs.uiuc.edu
>> *Subject:* Re: [LLVMdev] Does nounwind have semantics?
>>
>> On 22 July 2013 01:11, Kuperstein, Michael M
>> <michael.m.kuperstein at intel.com
>> <mailto:michael.m.kuperstein at intel.com>>
>> wrote:
>>
>> Of course frontends are free to put attributes, but it would be nice
>> if optimizations actually used them. ;-)
>> My use case is that of proprietary frontend that happens to know
>> some library function calls - which are only resolved at link time -
>> have no side effects and are safe to execute speculatively, and
>> wants to tell the optimizer it can move them around however it
>> likes. I'll gladly submit a patch that uses these hints, but I'd
>> like to reach some consensus on what the desired attributes actually
>> are first. The last thing I want is to add attributes that are only
>> useful to myself.
>>
>> Regarding having several orthogonal attributes vs. things like
>> "safetospeculate":
>>
>> To know a function is safe to speculatively execute, I need at least:
>> 1) readnone (readonly is insufficient, unless I know all accessed
>> pointers are valid)
>> 2) nounwind
>> 3) nolongjmp (I guess?)
>> 4) no undefined behavior. This includes things like "halting" and
>> "no division by zero", but that's not, by far, an exhaustive list.
>>
>> I guess there are several ways to handle (4).
>> Ideally, I agree with you, we'd like a set of orthogonal attributes
>> that, taken together, imply that the function's behavior is not
>> undefined.
>> But that requires mapping all sources of undefined behavior (I don't
>> think this is currently documented for LLVM IR, at least not in a
>> centralized fashion) and adding a very specific attribute for each
>> of them. I'm not sure having function declarations with "readnone,
>> nounwind, nolongjmp, halting, nodivbyzero, nopoisonval,
>> nocomparelabels, nounreachable, ..." is desirable.
>>
>> We could also have a "welldefined" attribute and a "halting"
>> attribute where "welldefined" subsumes "halting", if the specific
>> case of a function which halts but may have undefined behavior is
>> important.
>> While the two are not orthogonal, it's similar to the situation with
>> "readnone" and "readonly". Does that sound reasonable?
>>
>> You're entirely right. I forgot about undefined behaviour.
>>
>> If you want a 'speculatable' attribute, I would review that patch.
>> Please audit the intrinsics (at least the target-independent ones) and
>> appropriate library functions for whether you can apply this attribute
>> to them. I think the only optimization that it can trigger is that
>> "isSafeToSpeculativelyExecute" returns true on it. Anything else? Is
>> it safe to infer readnone and nounwind from speculatable?
>>
>> I should mention that speculatable functions are extraordinarily
>> limited in what they can do in the general (non-LLVM-as-a-library)
>> case. They may be hoisted above calls to fork or pthread_create, they
>> may be moved into global constructors (and thus can't depend on global state), etc.
>> However, since you have a specific library you want to generate code
>> against, you have the power to make use of it. I don't expect clang or
>> dragonegg to be able to make use of it.
>>
>> Nick
>>
>> -----Original Message-----
>> From: Nick Lewycky [mailto:nicholas at mxc.ca <mailto:nicholas at mxc.ca>]
>> Sent: Monday, July 22, 2013 10:24
>> To: Kuperstein, Michael M
>> Cc: Andrew Trick; llvmdev at cs.uiuc.edu <mailto:llvmdev at cs.uiuc.edu>
>> Subject: Re: [LLVMdev] Does nounwind have semantics?
>>
>> Kuperstein, Michael M wrote:
>>> I'm not sure I understand why it's blocked on that, by the way.
>>
>> It blocks our ability to automatically deduce the halting attribute
>> in the optimizer, which was necessary for the use case I had at the
>> time.
>> If you have a use case of your own, feel free to propose the patch!
>>
>> (Technically it's not *blocked* -- see how my patch does it! -- but
>> the workarounds are too horrible to be committed.)
>>
>>> Even if we can't apply the attribute ourselves, I don't see why
>> we wouldn't expose that ability to frontends.
>>
>> Frontends are free to put attributes on functions if they want to.
>> Go for it!
>>
>>> I'm not entirely sure "halting" is the right attribute either, by
>> the way.
>>> What I, personally, would like to see is a way to specify a
>> function call is safe to speculatively execute. That implies
>> readnone (not just readonly), nounwind, halting - and Eris knows
>> what else. Nick, is that too strong for you?
>>
>> I strongly prefer the approach of having orthogonal attributes.
>> There are optimizations that you can do with each of these
>> attributes on their own. In particular I think that
>> readonly+halting+nounwind+nolongjmp is going to be common and I'd
>> feel silly if we had a special case for
>> readnone+halting+nounwind+nolongjmp and thus couldn't optimize the more
>> common case.
>>
>> That said, I'm also going to feel silly if we don't end up with enough
>> attributes to allow isSafeToSpeculate to deduce it, which is where we
>> are right now. I was planning to get back to fixing this after
>> Chandler's promised PassManager work.
>>
>> Nick
>>
> ---------------------------------------------------------------------
> Intel Israel (74) Limited
>
> This e-mail and any attachments may contain confidential material for
> the sole use of the intended recipient(s). Any review or distribution
> by others is strictly prohibited. If you are not the intended
> recipient, please contact the sender and delete all copies.
> <speculatable.diff>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list