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

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a subscriber: hctim.
spatel added a comment.

In D105338#2865297 <https://reviews.llvm.org/D105338#2865297>, @jdoerfert wrote:

> In D105338#2864865 <https://reviews.llvm.org/D105338#2864865>, @spatel wrote:
>
>> 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?
>
> We pointed them to a fix (`__builtin_trap()`) a while ago, didn't we?

Yep, and now it's in place with (cc @hctim ):
D105654 <https://reviews.llvm.org/D105654> / d458f379324967 <https://reviews.llvm.org/rGd458f379324967c3c408be06e21aad9bc92c54cb>
...but there's a code comment and comment in PR47480 that says LLVM has a bug. I assume that means the LangRef has a bug, but nobody wants to change it?

As for Clang, I don't know how we'd solve that problem (without changing LLVM) other than to say (loudly) that the blog post no longer applies, and we want code to update. Now we can at least point people to the GWP-asan patch as a reference, but as noted, there is very likely going to be fallout...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105338