[PATCH] D20116: Add speculatable function attribute

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added a comment.

>>> So even if there is some way to detect the difference, I'd consider that a bug in the LLVM IR semantics since it disallows an important optimization.
>> 
>> I guess my basic point was exactly this - the semantics of our other attributes, compared to speculatable, are not so easy or non-shocking that i think artificially limiting speculatable to non-callsites only makes sense.
> 
> I'm not sure if I agree with the "artificially" characterization.  My objection was based around (what I think are) concrete problems that we'll have if we allow this.

Sorry, let me try to explain:
You have identified concrete problems.
IMHO, your concrete problems are not with the definition of the attribute, but instead are "things wanting to optimize callsites and keep this attribute have to understand control dependence".
But this is pretty much a truism to me given *any* sane definition of the attribute on callsites.
Additionally, IMHO, there can be  *no* sensible way to define this attribute such that the problems go away.
So from my perspective, the concrete problems you've identified are going to get solved in exactly the same way whether we add them for callsites today, tomorrow, or five years from now.
Hence i see the limitation as fairly artificial. We aren't going solve these problems other than by auditing all the callsite optimizations and either making them drop this attribute, or deeply understand control dependence enough to do a correct thing.

> As I said, my objection was based on my opinion that they're already discovered to be wrong.

I guess this is where we disagree.
I do not believe you have pointed out that there is something wrong with the semantic, at least in a way that you could solve.
I believe you have pointed out it will interact badly if we don't audit our code.

I'd be fine if our answer was "hey, start a patch with all the fixes necessary to make this work properly on callsites".
But otherwise, what's the path forward?
IE what do you see as a definition of these attributes on callsites that is sane but doesn't have the issues you foresee?


https://reviews.llvm.org/D20116