[PATCH] D87149: [InstCombine] erase instructions leading up to unreachable

[Nuno Lopes via Phabricator via llvm-commits]

nlopes added a comment.

> I would say let's write an RFC and see if there are other opinions. Also, @nlopes what does alive2 think of such a proposal?

Alive2 implements whatever semantics make sense for LLVM. We are not here to constrain the solutions, but to help :)
Right now Alive2 doesn't support volatile stores, and personally I haven't studied their semantics.

unreachable allows the compiler to delete any instruction that is executed beforehand as long as it returns. So, `noreturn` function calls and `__builtin_trap()` are stopping points.
Volatile stores if they are guaranteed to trap, then they should be considered as stopping point as well. I wouldn't know what to say about volatile stores that are guaranteed not to trap (i.e., when the compiler can prove the address is dereferenceable), but I'm inclined to say they can be removed, as UB allows externally observable behavior to be removed. If there's a compelling "niceness" argument because it's needed in practice, then we can keep volatile stores alone (e.g., are people writing `(volatile int*)0 = 0x42; __builtin_unreachable()` in linux kernel rather than `__builtin_trap()`??).
Regular stores can always be removed.

For example, this is ok:

  printf("foo")
  store 42, @glb
  icmp foo
  unreachable
    =>
  unreachable

I'm happy to implement that stricter volatile semantics (i.e., assume volatile accesses may trap) in Alive2 to help catching optimizations that are wrong w.r.t. to that "new" semantics.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87149/new/

https://reviews.llvm.org/D87149