[PATCH] D20116: Add speculatable function attribute

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

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

> In https://reviews.llvm.org/D20116#741087, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D20116#740922, @arsenm wrote:
> >
> > >
> >
> >
> > Why only for intrinsics? I thought we had concluded that we'd only allow it for declarations and not on call sites (which may technically mean on call sited but only matching the declaration). I think it is important that we can apply it to regular functions.
>
>
> I think a general speculatable attribute that is allowed only on functions decls is *less problematic*[0] that a context sensitive one, but I think speculatable intrinsics are clearly okay.  Therefore my opinion is (which I expressed on IRC to Matt) is to first land the intrinsic variant of this, since that's what he's blocked on; and then we can go ahead with more aggressive variants on subsequent patches.
>
> [0] https://reviews.llvm.org/D20116#709352


Okay, unfortunately, this is only useful to me if we allow it on function declarations, and I don't see how kicking this can down the road helps in this regard. If he adds this for intrinsics and I immediately turn around a propose a patch to remove the restriction, that's a waste of everyone's time. 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.


https://reviews.llvm.org/D20116