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

Thu May 23 22:05:38 PDT 2019

jdoerfert added a comment.

In D62199#1515159 <https://reviews.llvm.org/D62199#1515159>, @grokos wrote:

> Let me address what has been written so far from my perspective and request some changes since there has been demand for them:
>
> 1. 1, 4 and 5 represent my point of view, which is of course subjective, anyone could have a different mentality. But calling my reviews "reviews" and my activity here "suspicious" doesn't contribute at all. In pretty much the same way the "this is your problem, I've got the owner's approval" reply is inappropriate (and in no way do I endorse this "argument" as a reply to reviewers' requests).

I was not only referring to you, but it was not the right choice of words to begin with, you're right about that.
I will stick to arguments from now on. What I should have said is:

I think the following "NFC" commits are not "NFC" or the commit message is not matching: D61801 <https://reviews.llvm.org/D61801>, D61785 <https://reviews.llvm.org/D61785>, D61526 <https://reviews.llvm.org/D61526>, D61523 <https://reviews.llvm.org/D61523>, D56274 <https://reviews.llvm.org/D56274>
This is one of the commits that explain the problem they fix but do not provide a test: D56332 <https://reviews.llvm.org/D56332>
This commit message isn't explanatory and doesn't match the patch: D56290 <https://reviews.llvm.org/D56290>

And I am concerned because really quick LGTM for non-trivial changes happened before (not including this one or any of the aforementioned), e.g.,
in D53141 <https://reviews.llvm.org/D53141> @Hahnfeld pointed out problems.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62199