[Openmp-commits] [PATCH] D62393: [OPENMP][NVPTX]Mark parallel level counter as volatile.

Mon Oct 7 16:46:05 PDT 2019

ABataev added a comment.

In D62393#1698531 <https://reviews.llvm.org/D62393#1698531>, @JonChesterfield wrote:

> > as tot he volatile modifi3r, read the whole thread. There was a big discussion about it.
>
> I have read the thread. There's a long discussion about volatile vs atomic. There is no justification for marking the accesses volatile via asm instead of marking the byte array as volatile.

It means that you missed something. Initially, patch marked the whole array as volatile but after some discussion it was decided to use ptx instructions directly to avoid some side effects.

> The semantics of ptx volatile are stronger than those of llvm. Weird, but not a justification for omitting the volatile qualifier from the C in this change.
> 
> Assuming llvm doesn't spuriously drop volatile (which would be a severe bug), and that the lowering to ptx doesn't drop volatile (which also seems likely to be fatal), a volatile array in C would lead to the same memory operations introduced by the asm. That's why I thought it was the memory clobber that mattered.

Repository:
  rOMP OpenMP

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

https://reviews.llvm.org/D62393