[PATCH] D79507: [flang] Change DIE("unreachable") cases to use llvm_unreachable

Thu May 28 14:53:00 PDT 2020

DavidTruby added a comment.

In D79507#2061428 <https://reviews.llvm.org/D79507#2061428>, @klausler wrote:

> In D79507#2055514 <https://reviews.llvm.org/D79507#2055514>, @DavidTruby wrote:
>
> > The runtime behaviour when compiled with RELEASE is undefined. As I and @hfinkel mentioned, the point of that is optimisations that cannot otherwise be made by the compiler. I would say that runtime performance of release builds should be one of our main priorities...
>
>
> Release builds of the compiler must have well-defined behavior, and that behavior must be to crash on a fatal internal error rather than to silently proceed to generate incorrect code, which is the worst thing a compiler can do to a user. 
> Trading off protection against silent generation of bad code to gain some unmeasured performance improvement would be insanely irresponsible.

If this is the case then we shouldn't be writing this in a language that frequently explicitly leaves things as undefined behaviour for performance reasons, which is something that both the C and C++ standards do. Undefined behaviour is both unavoidable in C++ and necessary for many of the optimisations that C++ compilers perform. Given the language we're writing in it is //never// going to be possible to have fully well defined behaviour for every runtime path in the compiler.

Regardless, as I've said before in this thread, there is no undefined behaviour in any of these cases; none of these macro calls are actually reachable; in some cases this is trivial, in some cases this is asserted by comments already and so those comments should be changed if this is not true. We're simply attempting to signal that to the compiler in cases that it cannot necessarily prove it itself, so that it can perform optimisations based on that fact.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79507