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

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added a comment.

In D62199#1512697 <https://reviews.llvm.org/D62199#1512697>, @arsenm wrote:

> In D62199#1512638 <https://reviews.llvm.org/D62199#1512638>, @ABataev wrote:
>
> > 1. Because it is impossible to test them, that why it is NFC.
>
>
> This doesn't make it NFC. There is intended change in behavior.


In beehavior of the compiler, but not the code itself.

> 
> 
>> 2. Yes, this is true. Bu the changes are rather small.
> 
> This isn't a reason to merge unrelated patches

Yes, I agree , it would be better to split this patch into 2 parts.

>> 3. It is only your opinion.
> 
> I agree with this issue. This is very hand wavy. If you cannot describe what the undefined behavior or problem is is, you can't come up with a sensible solution. As presented here, this does not justify why this fixes or works around the issue.

It is related somehow with the ptxas translator. I cannot explain why it causes the problem, but in some rare cases, when you mix simultaneos access and the atomic operations, the code is optimized in the unknown way and it leads to undefined behavior.

> 
> 
>> 4. Again, this is your problem, The patch was approved bythe code owner
> 
> Patches can still be subject to post-commit reviews

Sure.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62199