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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D105338#2863990 <https://reviews.llvm.org/D105338#2863990>, @chandlerc wrote:

> But just reverting this is pretty hostile unless Clang has been updated to make C and C++ volatile stores continue to behave as the language expects first.
>
> Just regressing Clang because no one has written the LangRef change doesn't make sense to me. I think you should fix Clang to not be buggy first if you want to pursue this (or work with the Clang devs to get such a patch), just like we would fix a buggy LLVM pass that broke real users before enabling a valid optimization that uncovered the big.

I agree. I don't know exactly what fixes are needed for Clang, but we do know that code like this:
https://github.com/llvm/llvm-project/blob/8cf60e61e7b0e3213b5b81039878dcf1ef18c00f/compiler-rt/lib/gwp_asan/guarded_pool_allocator.cpp#L254
( the root cause of https://llvm.org/PR47480 ) is going to break again with this change.

If the goal is to get source like that to change, then we might as well fix that first, so we can point to it as a reference example of how to fix code?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105338