[PATCH] D147719: [OpenMP] Replace HeapToShared's initial value with `poison`

Sat Apr 8 07:48:57 PDT 2023

nlopes added a comment.

In D147719#4253168 <https://reviews.llvm.org/D147719#4253168>, @jhuber6 wrote:

> In D147719#4253166 <https://reviews.llvm.org/D147719#4253166>, @nlopes wrote:
>
>> I've no clue about these shared variables.
>>
>> If you require that there's a store before any load, then the initialization value doesn't matter and can (should) be poison.
>> If it's ok to do an uninitialized of these shared variables, then you need to decide what's the semantics you want. If you want that comparison in the example above to be executed, poison is not ok.
>> Note that in C++, reading an uninitialized integer is UB, so you can use poison safely.
>
> This optimization specifically replaces local C++ variables like in this example https://godbolt.org/z/hfM4b35q9. I think this is a valid transformation at least in this optimization at least because it targets local variables. I think the other discussions about how we handle the threading concepts are unrelated to this patch.

local vars are ok to be poison on uninit, yes. So this patch looks good to me.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147719