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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

In D87149#2264099 <https://reviews.llvm.org/D87149#2264099>, @chandlerc wrote:

> Regarding access to volatile memory...
>
> If volatile memory access is known to not trap, then the guidance on LLVM's own blog is wrong:
> https://blog.llvm.org/posts/2011-05-13-what-every-c-programmer-should-know/

To be precise, I think the entire paragraph (below) is not correct (from todays perspective). The flag exists (in IR) and the behavior of volatile accesses, e.g. of `nullptr`, is target specific (in IR).

>> If you're using an LLVM-based compiler, you can dereference a "volatile" null pointer to get a crash if that's what you're looking for, since volatile loads and stores are generally not touched by the optimizer. There is currently no flag that enables random NULL pointer loads to be treated as valid accesses or to make random loads know that their pointer is "allowed to be null".



> And in fact, Clang's error message is wrong as it specifically suggests re-writing a dereference of a null pointer to use volatile in order to ensure a trap.

We should have a trap intrinsic for that, and I thought we do actually... let's be honest, that is also a way nicer way to ensure a trap so we should advertise it either way.

> If we insist on following the LangRef rules, we would need to change the lowering of volatile memory accesses in Clang, and that doesn't seem especially useful. I think the LangRef is simply wrong on this point, and we need to admit that access to volatile memory, much like a call to an unknown external function, may trap, unwind, never return, etc. There is a significant amount of real C and C++ code written that relies on these rules, no doubt because LLVM and Clang advertised them. =]

I would argue to scrub

>> The compiler may assume execution will continue after a volatile operation, so operations which modify memory or may have undefined behavior can be hoisted past a volatile operation.

from the LangRef and to teach `isGuaranteedToTransferExecutionToSuccessor` about this is the best option here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87149