[PATCH] D20116: Add speculatable function attribute

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

In https://reviews.llvm.org/D20116#741181, @sanjoy wrote:

> In https://reviews.llvm.org/D20116#741170, @hfinkel wrote:
>
> > Okay, unfortunately, this is only useful to me if we allow it on function declarations
>
>
> I had somehow missed this bit ^ and I was under the impression that the main motivation for a general attribute was more completeness than anything else.


No problem.

> 
> 
>> I thought that we had agreed that allowing it on function declarations was okay so long as we documented the fact that this introduces potential UB just by declaring such a function, so let's do that.
> 
> I had not phrased my concession clearly. :)
> 
> Just to be clear, I don't think they're **okay**, but I can live with them in the spirit of begin pragmatic.

I understand. In a theoretical sense, I see adding them on function declarations as the same as adding them to intrinsics. Obviously there are practical differences, however, I'm not sure that in practice you're more likely to introduce a call to an arbitrary function, just because it happens to have been declared as speculatable, than you are to an intrinsic, just because it happens to be similarly available. I can't imagine any general transformation doing anything for each declared function just on the basis of it being declared. You'd need to be operating in a very restricted environment for that to make sense, and in such an environment, you should reasonably have the power not to mark functions as speculatable in a problematic way.

> So yes, if this attribute will be useless to you without the generalization to non-intrinsics, then I won't object to checking in the previous version of this patch.

Thanks!


https://reviews.llvm.org/D20116