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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D79507#2056617 <https://reviews.llvm.org/D79507#2056617>, @sscalpone wrote:

> +1 for good messages.
>
> I think there's universal agreement that code that cannot be reached with user input should be marked as unreachable.  The point of contention is whether or not code in a RELEASE build is allowed to catch & report that situation.
>
> What about something like:
>
>   #define DIE(x) { Fortran::common::die(x " at " __FILE__ "(%d)", __LINE__); LLVM_BUILTIN_UNREACHABLE; }
>
>
> ?


I'd generally consider that inconsistent with LLVM's style and what I was trying to avoid by having the discussion about this issue prior to flang being integrated into the project - I think it'd be a hinderance to the LLVM project if the unreachable-equivalent were always-on in release builds & thus had different performance characteristics (and then need for an extra unreachable taxonomy - those cheap enough to keep in release builds and those that are too expensive). LLVM's "expensive checks" is already an awkward extra variant/build mode - I think it's important we don't create more of those if possible.

Is there reason to believe that a subset of asserts/unreachable that would be always-on would improve Clang significantly, for instance? If not, then I don't think flang should be making a different tack here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79507