[PATCH] D87149: [InstCombine] erase instructions leading up to unreachable

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D87149#2263610 <https://reviews.llvm.org/D87149#2263610>, @stephan.yichao.zhao wrote:

> This change affected a case like this:
>
>   code with side effects, for example, volatile stores
>   an unreachable instruction __builtin_unreachable
>
> Although executing __builtin_unreachable is undefined, removing the code before it deletes their side effects.

Oh, hm.
While `volatile` stores aren't a great example, we certainly shouldn't be dropping e.g. `printf()` calls.

> Can we temporarily revert this change and add checking side effect checking for code to be removed?

It doesn't appear we have such a check handy (`isAssumedSideEffectFree()` in Attributor seems like the best match),
so i guess it would be fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87149