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

[Tim Keith via Phabricator via llvm-commits]

tskeith added a comment.

> In release mode, where we do care about the performance of the Compiler's -fsyntax-only mode, these are compiled out allowing for more-aggressive optimization.

This is the crux of my concerns. It has been determined, presumably by measurement, that clang's -fsyntax-only benefits enough from this optimization that it is worth the cost of a worse user experience when the internal error occurs (undefined behavior vs. internal error message with file and line number). There is no evidence that changing any occurrence of `DIE` in flang would have similar benefits. It's simply premature optimization.

This optimization decision should be up to the programmer, not a project-wide policy. Now in flang we have the choice of reporting an internal error with `DIE` when it's low-cost to be triggered in a Release build, or `llvm_unreachable` for those (IMO rare) cases where you know there is a worthwhile performance benefit. Similar to `std::vector::at` vs. `std::vector::operator[]`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79507