[llvm] 4e413e1 - [InstCombine] Temporarily do not drop volatile stores before unreachable

[Matt Arsenault via llvm-commits]

> On Sep 10, 2020, at 10:18, Nikita Popov via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> 
> 
> Author: Nikita Popov
> Date: 2020-09-10T16:16:44+02:00
> New Revision: 4e413e16216d0c94ada2171f3c59e0a85f4fa4b6
> 
> URL: https://github.com/llvm/llvm-project/commit/4e413e16216d0c94ada2171f3c59e0a85f4fa4b6
> DIFF: https://github.com/llvm/llvm-project/commit/4e413e16216d0c94ada2171f3c59e0a85f4fa4b6.diff
> 
> LOG: [InstCombine] Temporarily do not drop volatile stores before unreachable
> 
> See discussion in D87149. Dropping volatile stores here is legal
> per LLVM semantics, but causes issues for real code and may result
> in a change to LLVM volatile semantics. Temporarily treat volatile
> stores as "not guaranteed to transfer execution" in just this place,
> until this issue has been resolved.
> 
> Added: 
> 
> 
> Modified: 
>    llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
>    llvm/test/Transforms/InstCombine/volatile_store.ll
> 
> Removed: 
> 
> 
> 
> ################################################################################
> diff  --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
> index 0ca256860c59..63ba7eb85c66 100644
> --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
> +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
> @@ -2805,6 +2805,14 @@ Instruction *InstCombinerImpl::visitUnreachableInst(UnreachableInst &I) {
>   Instruction *Prev = I.getPrevNonDebugInstruction();
>   if (Prev && !Prev->isEHPad() &&
>       isGuaranteedToTransferExecutionToSuccessor(Prev)) {
> +    // Temporarily disable removal of volatile stores preceding unreachable,
> +    // pending a potential LangRef change permitting volatile stores to trap.
> +    // TODO: Either remove this code, or properly integrate the check into
> +    // isGuaranteedToTransferExecutionToSuccessor().
> +    if (auto *SI = dyn_cast<StoreInst>(Prev))
> +      if (SI->isVolatile())
> +        return nullptr;

This misses atomics and volatile memcpy/memset and co. I think there’s a helper somewhere to check all of them