[PATCH] D20116: Add speculatable function attribute

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added a comment.

I can phrase my concerns as a question -- what is the intended behavior of the following program if `condition` is `false`?

  void f(int a, int b) { return a / b; }
  
  void main() {
    if (condition)
      f(5, 0) speculatable;
  }

As far as I can tell, there are two possibilities:

1. It is well defined -- `main` is a no-op.
2. It has undefined behavior, due to the incorrect `speculatable` attribute.

If we go with (1), then we cannot hoist the call to `f` above the `if` -- we'd introduce UB if we do that.  This is the interpretation I'm leaning towards, but this interpretation makes the `speculatable` attribute useless for general functions, since we're prevented from doing what it was supposed to enable in the first place.

If we go with (2), then we've admitted that "dead code" (that is, code that is not run) can influence program behavior, which I find troubling.  In particular, I think `foo1` and `foo2` should be equivalent:

  void foo1() {
  }
  
  void foo2() {
    if (false) {
      // Arbitrary syntactically valid stuff
    }
  }

In other words, adding dead code should not change program behavior.

Even

  void f(int a, int b) speculatable { return a / b; }
  
  void main() {
    if (condition)
      f(5, 0);
  }

has the same problem, so I take back my earlier concession on function annotations.

Now it is fine for us to have a `speculatable` **analysis** which would infer `speculatable` in limited cases, like `int f() { return 10; }`.  But that's a module analysis, and not an attribute.
Having them on intrinsics is fine too, since we axiomatically know that certain intrinsics are speculatable, just like we know the `add` instruction is speculatable.


https://reviews.llvm.org/D20116