[PATCH] D20116: Add speculatable function attribute

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

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?


https://reviews.llvm.org/D20116