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

[Hal Finkel via Phabricator via llvm-commits]

hfinkel 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.


To be clear, we're all experienced compiler developers here producing production software. No one is suggesting anything that would be "insanely irresponsible." Obviously, we make all kinds of trade-offs here for performance, and correctness is simultaneously critically important, and we have general coding conventions that favor for performance understanding that the integrated effect of microoptimizations matters, even if the individual effect of every individual choice is practically unmeasurable. We use vector's `operator []` instead of `at()` with exceptions, for example. While silently generating incorrect code is clearly unacceptable, the use of `llvm_unreachable` is one of many cases where we depend on internal invariants and, in places where risk is high, we should have better testing and restructure the code to help ensure correctness.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79507