[PATCH] D106299: [Local] Flag to decide if a trap should be placed before unreachable

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:2351
                                    SI->getPointerAddressSpace()))) {
-          changeToUnreachable(SI, true, false, DTU);
+          changeToUnreachable(SI, /* PreferToUseLLVMTrap */ true, false, DTU);
           Changed = true;
----------------
This (and the corresponding handling for calls, below) is the only place where we're passing PreferToUseLLVMTrap=true, right?

It looks like there was an attempt to remove the trap generation back in 2016, but it got reverted.  Revert said it "it seems to break UBSan".  (rGe14e7bc4b889dfaffb7180d176a03311df2d4ae6).  Maybe worth attempting again; we really shouldn't need to insert a trap after non-volatile store to null.

(Note this has nothing to do with the backend converting an "unreachable" to a trap instruction. Some backends do this for various reasons. For example, on Windows, it's necessary to generate correct EH tables in some cases, I think.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106299