[all-commits] [llvm/llvm-project] ffb327: [SimplifyCFG] Improve store speculation check

[Nikita Popov via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: ffb3277b0036909a8e622d5758a1e2850eabfd19
      https://github.com/llvm/llvm-project/commit/ffb3277b0036909a8e622d5758a1e2850eabfd19
  Author: Nikita Popov <nikita.ppv at gmail.com>
  Date:   2021-07-26 (Mon, 26 Jul 2021)

  Changed paths:
    M llvm/lib/Transforms/Utils/SimplifyCFG.cpp
    M llvm/test/Transforms/SimplifyCFG/speculate-store.ll

  Log Message:
  -----------
  [SimplifyCFG] Improve store speculation check

isSafeToSpeculateStore() looks for a preceding store to the same
location to make sure that introducing a new store of the same
value is safe. It currently bails on intervening mayHaveSideEffect()
instructions. However, I believe just checking mayWriteToMemory()
is sufficient there -- we just need to make sure that we know which
value was stored, we don't care if we can unwind in the meantime.

While looking into this, I started having some doubts about the
correctness of the transform with regard to thread safety. While
we don't try to hoist non-simple stores, I believe we also need
to make sure that the preceding store is simple as well. Otherwise
we could introduce a spurious non-atomic write after an atomic write
-- under our memory model this would result in a subsequent undef
atomic read, even if the second write stores the same value as the
first.

Example: https://alive2.llvm.org/ce/z/q_3YAL

Differential Revision: https://reviews.llvm.org/D106742