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

[Alexey Bataev via Phabricator via Openmp-commits]

ABataev added a comment.

In D62199#1517522 <https://reviews.llvm.org/D62199#1517522>, @Hahnfeld wrote:

> In D62199#1517460 <https://reviews.llvm.org/D62199#1517460>, @ABataev wrote:
>
> > In D62199#1517437 <https://reviews.llvm.org/D62199#1517437>, @hfinkel wrote:
> >
> > > Alexey, I believe that I understand the distinction that you are making. That's not the way the term is used within the LLVM community. The observable behavior of the compiler and its runtime libraries is what matters. The fact that some implementation technique should not be required to make that observable behavior correct is irrelevant in our usage. The introduction of that technique in order to make the observable behavior correct, where it was not previously correct, is a functionality-changing change to the compiler. Thus, it is not NFC.
> >
> >
> > It is better to express this explicitly in the policy, because it differs from the classical definition.
>
>
> See https://llvm.org/docs/Lexicon.html#nfc:
>
> > “No functional change”. Used in a commit message to indicate that a patch is a pure refactoring/cleanup. Usually used in the first line, so it is visible without opening the actual commit email.


This is my last comment. I'm not going yo continue this discussion anymore. 
Almost all my patches marked as NFC are refactoring patches. So, the patches were correctly marked as NFC per the definition in the lexicon. Thanks for the reference, Jonas. 
If you don't think that "volatile" is a refactoring, I'm sorry. Anyway, the patch were reverted and split into 3 parts.

> AFAICS that has nothing to do with "non-functional" requirements. [ We could continue arguing about "implementation constraints" that fix bugs in certain situations being related to functional or non-functional requirements, but this is really not the question. ]

Sorry, you're wrong. The programming is the work with requirements expressed explicitly or implicitly. and all changes in code are to implement those requirements or to fix bugs/refactor the code in accordance with the requirements. So, the changes are also functional and non-functional. If the patch explicitly changes the logical dependency between inputs and outputs, this is a functional change. Otherwise, the change is non-functional. It may be a refactoring or a fix for non-functional requirement/constraint.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D62199