[PATCH] D105338: [InstCombine] Revert "Temporarily do not drop volatile stores before unreachable"

[Nico Weber via Phabricator via llvm-commits]

thakis added a comment.

Possible paths forward:

1. Make clang lower volatile literal nullptr derefs to `__builtin_abort`, land LLVM change as-is
  - Maybe add option to control whether to do this lowering or not
  - Advantages:
    - Keeps user code working as expected without work on part of users
    - LLVM change can go in quickly
  - Disadvantages:
    - Embedded folks might want to have a way to write to 0?
    - Most of LLVM's "nullptr is magic" logic probably exists for C, so it's a bit strange to make this not fire for C frontends
    - Other frontends still exposed to the change, but some of them (Rust, Swift) probably rely on nullptrs less. Not sure about Fortran

2. Make clang diag volatile literal nullptr derefs and ask users to migrate all their code, then land LLVM some appropriate time later

3. *handwave* Make langref self-consistent (which seems to be the motivation for this change? Still not quite sure) by making other cases less aggressive instead of this case more aggressive

4. Keep things as is, update some documentation

>From an uninformed distance (1) and (4) seem alright.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105338