[PATCH] D107281: [SimpifyCFG] Speculate a store preceded by a local non-escaping load

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D107281#2922901 <https://reviews.llvm.org/D107281#2922901>, @chill wrote:

> In D107281#2922685 <https://reviews.llvm.org/D107281#2922685>, @lebedev.ri wrote:
>
>> I'm afraid i'm not seeing why `@load_before_store_escape` must not be transformed.
>> Alive2 says it's fine: https://alive2.llvm.org/ce/z/F7ctum
>> I'm not seeing any syncronization primitives, so i'm not really seeing yet why we'd be introducing a race?
>
> Once we've made the address of the local variable visible to other functions, we cannot discard the possibility that those other functions may spawn threads
> that write to the variable. In the original program, a data race might have been prevented by the condition, but once we move the store outside the
> condition, we must be sure a data race wasn't possible anyway, no matter what the condition evaluates to.
>
> Seeing just a load does not guarantee absence of a data race (unlike if we see a store) - the load may still be a part of a race, just not being UB (cf. https://llvm.org/docs/Atomics.html#optimization-outside-atomic)

Forgot to say, please add something along those lines at least into the patch description.


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

https://reviews.llvm.org/D107281