[Openmp-commits] [PATCH] D62199: [OPENMP][NVPTX]Fix barriers and parallel level counters, NFC.

Johannes Doerfert via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Wed May 22 13:01:46 PDT 2019


jdoerfert reopened this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

I have a problem with this patch and this kind of patches/reviews:

1. How can a "Fix" commit be "NFC" at the same time?
2. The commit message basically mentions that multiple unrelated changes are combined in one patch.
3. I somehow doubt that it's the "ptx optimizations" that "lead to UB":

> Parallel level counter should be volatile to prevent some dangerous optimiations by the ptxas. Otherwise, ptxas optimizations lead to undefined behaviour in some cases.

This does not sound like a "solution" but more like a hack. With `volatile` the problem goes away so let's make it `volatile`.

4. The more patches go through with this kind of "review" the more "suspicious" this looks to me.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62199





More information about the Openmp-commits mailing list