[PATCH] D20116: Add speculatable function attribute

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a subscriber: whitequark.
sanjoy added a comment.

In https://reviews.llvm.org/D20116#708268, @hfinkel wrote:

> > It also breaks "code compression" type optimizations:
> > 
> >   if (a == 1 || a == 2) {
> >     switch (a) {
> >     case 1:
> >       div(m, 1) speculatable;
> >       break;
> >     case 2:
> >       div(m, 2) speculatable;
> >       break;
> >     }
> >   }
> > 
> > 
> > to
> > 
> >   if (a == 1 || a == 2) {
> >     div(m, a) speculatable;
> >   }
> > 
> > 
> > to
> > 
> >   if (a == 1 || a == 2) {
> >     if (a == 0)
> >       div(m, 0) speculatable;
>
> Your "code compression" optimization just introduced dead code ;)


Yea, I don't even know why I called it "code compression". :)

> 
> 
>>   else
>>     div(m, a) speculatable;
>> 
>> }
>> 
>>   
> 
> I think that all of this is right, you can't apply some of these optimizations to call sites with the speculatable attribute. I agree, however, that we need to think carefully about how to define what speculatable means on an individual call site. Perhaps they're like convergent functions in this regard: you can't introduce new control dependencies (at least not in general).

I'd say as an initial step support for intrinsics that we _know_ are `speculatable` (like we _know_ that `add` is speculatable) can land without any further discussion.

As for a generic `speculatable` attribute -- I need some time to think about this.  Perhaps if done with sufficient care, it is possible.

I'd also like to ping @whitequark for comments.  I had blocked https://reviews.llvm.org/D18738 some time back on similar grounds, but if we can reach a conclusion on `specuatable`, perhaps some of that can be transferable to `!unconditionally_dereferenceable` as well.


https://reviews.llvm.org/D20116