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

JF Bastien via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Tue Jun 11 16:07:51 PDT 2019


jfb added a comment.

In D62393#1539040 <https://reviews.llvm.org/D62393#1539040>, @hfinkel wrote:

> In D62393#1539014 <https://reviews.llvm.org/D62393#1539014>, @ABataev wrote:
>
> > In D62393#1539012 <https://reviews.llvm.org/D62393#1539012>, @jfb wrote:
> >
> > > FWIW we already support `-fms-volatile`, so there's precedent if you wanted `-fnv-volatile` (however terrible that is).
> >
> >
> > Most probably, we don't need this option, clang should emit correct code for volatile vars in Cuda mode automatically.
>
>
> +1
>
> That having been said, maybe this is a very simple change because it is the same as (or very similar to) what MSVolatile does?


My point exactly: MS volatile promotes volatile to `seq_cst`, it's trivial to copy and promote to `relaxed` and isn't too objectionable because we've got precedent.

Further consideration: you say you want to match the nvcc semantics... but it sounds like it doesn't actually have great / documented semantics, and they might changes. Should this `volatile`->`relaxed` be opt-in or opt-out? Is that the only thing we'd want to guarantee?


Repository:
  rOMP OpenMP

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

https://reviews.llvm.org/D62393





More information about the Openmp-commits mailing list