Reconsider llvm_unreachable usage: audit, policy, alternatives

[dtzSiFive]

llvm_unreachable in release builds instructs the compiler that it is, in fact, unreachable (UB if reached). As a result, this can result in rather unexpected situations if it ever becomes reachable, causing compiler to omit bounds checks or always take a branch knowing it 'must' be true.

Recently this caused a user to be confused when presented with a diagnostic describing a problem obviously not relevant for the code in question, which is not a good experience to provide.

The justification for this behavior is performance, and while that is a serious concern we may wish to consider our usage to be sure this is what we want.

One approach is we require/prefer the use of the cmake option that converts these to a trap in Release (see below). This is straightforward, hardens occurrences of this in LLVM itself (which personally I'm less concerned about), and easy to deploy. Personally, I would prefer our code worked regardless of build option, especially those that are not the default.

If keeping these, perhaps we only use this in cases where it's very much guaranteed and ideally locally obvious that the path in question is and always will be unreachable (for example, after a covered switch statement, and only because our compiler warns if we miss one). Basically, audit+policy(+ideally checked automatically, but don't have story there).

If not keeping these, they could be assertions instead with some code for that return path, or hopefully in many cases could be converted to emit proper diagnostics (perhaps we could have our own llvm_unreachable alternative that does this for us)? (good parts of this idea should be credited to @fabianschuiki :smile:).

Discourse discussion about this for some more detail: https://discourse.llvm.org/t/llvm-unreachable-is-widely-misused/60587/23 .

LLVM CMake option for converting these to trap: https://github.com/llvm/llvm-project/commit/6316129e066e . FWIW the IREE project replaced these in their code (see discourse for link, don't want to ping them here).