[PATCH] D121750: Add a cmake flag to turn `llvm_unreachable()` into builtin_trap() when assertions are disabled

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: llvm/CMakeLists.txt:468
 option(LLVM_ENABLE_DUMP "Enable dump functions even when assertions are disabled" OFF)
+option(LLVM_UNREACHABLE_OPTIMIZE "Whether llvm_unreachable() should be considered undefined behavior (default) or instead trap when assertions are disabled" ON)
 
----------------
dexonsmith wrote:
> dexonsmith wrote:
> > I wonder if the wording should be tied to the word "optimize". Also, note that trapping is a legit implementation of undefined behaviour; so another interpretation is that `llvm_unreachable()` is being treated as UB either way.
> > 
> > E.g.:
> > 
> > > A "reached" llvm_unreachable() is undefined behaviour; ON optimizes the unreachable control flow (default), while OFF triggers a guaranteed trap
> > 
> > WDYT?
> > (I'm okay with landing the wording as you had it if you prefer it; can always update it later)
> Or maybe this should be short and sweet:
> 
> > Optimize control flow based on llvm_unreachable(); guaranteed trap when OFF
> 
> (and leave the longer explanation for llvm/docs/...)
Trapping is a legit implementation of UB, but the opposite isn't and this is the important bit I was trying to express. 
I see what you mean with "optimize" but it may not be obvious for everyone that we're turning a trap into UB here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121750