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

Artem Belevich via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 7 13:33:17 PDT 2023


tra added a comment.

In D147719#4251944 <https://reviews.llvm.org/D147719#4251944>, @jdoerfert wrote:

> Your example is not impacted by the Undef -> Poison change. What you describe is either a valid transformation for both, or invalid for both.

Are they not impacted because it "happens to work" now, or because both undef and poison are guaranteed to do the right thing here? That's what I'm unsure about, as poison sounds like it would give the compiler more freedom to optimize, while I'm afraid that it may already have too much freedom as it is.

> The example from before cannot be optimized as is, but if you make `x` internal, we already propagate the value just fine (with undef as initializer):
> https://godbolt.org/z/bo3zn56jM

Exactly. That's where GPUs throw a wrench in LLVM's view of the world. While that variable may be internal to the module, as far as the symbol visibility is concerned, it is accessible from the other threads and may be modified by them.
The example above produces wrong code as nothing ever will be stored in `x`, before we load from it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147719



More information about the llvm-commits mailing list