[PATCH] D65377: [FunctionAttrs] Annotate intrinsics with nosync

Hal Finkel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 2 16:29:39 PDT 2019


hfinkel added a comment.

In D65377#1613024 <https://reviews.llvm.org/D65377#1613024>, @jdoerfert wrote:

> In D65377#1612906 <https://reviews.llvm.org/D65377#1612906>, @arsenm wrote:
>
> > In D65377#1610798 <https://reviews.llvm.org/D65377#1610798>, @jdoerfert wrote:
> >
> > > In D65377#1610775 <https://reviews.llvm.org/D65377#1610775>, @arsenm wrote:
> > >
> > > > I almost think intrinsics should be assumed nosync by default. I'm not thrilled at the prospect of another attribute that nearly every intrinsic needs to be annotated with
> > >
> > >
> > > Mh, I'm not opposing that idea but also not convinced. Doing it for all makes it easier to introduce errors I guess. Do we have precedence?
> > >
> > > If we want to do this, maybe we should make a list of attributes that need to be explicitly removed for intrinsics.
> > >  I expect not only `nosync` to be on it but also others like `willreturn`, `nofree`, `nounwind`, etc.
> >
> >
> > It probably should be explicit, but I feel like I'm in a perpetual state of adding attributes to intrinsic definitions and it's never complete
>
>
> I would propose the following:
>
> 1. Let's go ahead with this
> 2. We ask on the list how people feel about "opt-out" attributes for intrinsics and go from there


I agree. FWIW, I'd be more comfortable with some TableGen way of defining common groups of properties, giving these groups a single name, and then use that property-group name on most of the intrinscs.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65377/new/

https://reviews.llvm.org/D65377





More information about the llvm-commits mailing list