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

[Alexey Bataev via Phabricator via Openmp-commits]

ABataev added a comment.

In D62393#1538350 <https://reviews.llvm.org/D62393#1538350>, @jfb wrote:

> >> I think it is important to note that, to the best of my knowledge, no one was implying any bad faith on the part of anyone. To name one specific factor: We have had problems in the past where groups of collaborators/coworkers have done off-list reviews, followed only by a perfunctory/superficial on-list review, resulting in a lack of public discussion around the relevant design and implementation considerations. The review history highlighted by Johannes can give that impression, and we all need the community to watch itself in this regard, because of many potentially-relevant factors, to ensure the quality of our public code reviews is high. I see no reason to believe that everyone here wants to create the best possible code base. Sometimes that means one member of the community pointing out that, in his or her opinion, past reviews might have been insufficiently considered, and we should welcome that, and all work together to address these kinds of concerns going forward.
> > 
> > Hal, I agree that it was very dangerous situations, but I think, you need to prove something before trying to escalate the situation and blame somebody in doing the incorrect things. Johannes did this without any proves, he directly blamed me and others in doing improper things. Though he has no idea at all how things work in Cuda.
>
> Alexey: Hal is trying to turn this review into a productive one. Please internalize his guidance, and come back ready to answer feedback which you'd rather not answer. I don't see this review going anywhere without you changing your approach. You're frustrated by having to explain what you consider to be basics, and that's fine. Say so politely, and point at relevant resources. Reviews don't *need* to be a school lecture, but an open-source project is healthy when everyone learns. First it forces the committer to actually understand what they're doing (explaining basics is a great learning tool), and second it makes the code easier to maintain if the original committer stops participating.


I explained already everything several times. I said explicitly that in Cuda volatile is a little bit different than in C/C++. I explained, that volatile operations has the same semantics as relaxed atomic per PTX ISA. What else I could say? To explain how parallelLevel counter works I need to explain how the warp works. The reviewer need to know it himself. After he gets this basic knowledge,  the main idea will be clear. If not, I can help to understand it. The code of the runtime is not a kind of rocket science, actually it is very simple. But you need to spend some time yourself.


Repository:
  rOMP OpenMP

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

https://reviews.llvm.org/D62393