[PATCH] D20116: Add speculatable function attribute

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

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

> In https://reviews.llvm.org/D20116#706969, @arsenm wrote:
>
> > In https://reviews.llvm.org/D20116#706967, @sanjoy wrote:
> >
> > > I'm okay with this as long as this is allowed only on specific intrinsics (i.e. it cannot be used by external clients, but we //know// that certain intrinsics are speculatable).  I don't think we can allow this as generic function attribute, since that allows dead code to affect program behavior.  E.g.:
> > >
> > >   if (false)
> > >     puts("hi") speculatable;
> > >
> > >
> > > which can get transformed to
> > >
> > >   puts("hi") speculatable;
> > >   if (false)
> > >     ;
> > >
> > >
> > > The `speculatable` attribute on `puts` is incorrect, but we need to allow such "bogus" IR down dead paths.  For instance, the original program could have been:
> > >
> > >   void do_call(fnptr f, bool is_speculatable) {
> > >     if (is_speculatable)
> > >       f("hi") speculatable;
> > >     else
> > >       f("hi");
> > >   }
> > >  
> > >   // Later
> > >   do_call(puts, false);
> > >
> >
> >
> > Then the program is broken to begin with? Why shouldn't speculatable apply to functions that only call other speculatable instructions, similar to how FunctionAttrs can infer this for readnone? Do you mean not expose a user visible attribute in the frontend?
>
>
> `readnone` etc. are different from `speculatable`, in that once you mark a call site as `speculatable` you've the said call site as speculatable throughout the lifetime of the program (since, by definition, it can be arbitrarily speculated).  `readnone`, `readonly` etc. do not have that property.
>
> But thinking about it a bit, I concede that `speculatable` as a generic (i.e. both intrinsic and non-intrinsic) function attribute is fine.  However, it doesn't make sense as a call site attribute:  being speculatable only down a control flow path is basically the antithesis of `speculatable`.


True, but it can still be a callsite attribute. It can represent a data dependency:

  int div(int a, int b) {
    return a/b;
  }
  
  div(q, 1); /* speculatable */


https://reviews.llvm.org/D20116