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

Alexey Bataev via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Fri May 24 19:21:22 PDT 2019

ABataev added a comment.

In D62199#1517078 <https://reviews.llvm.org/D62199#1517078>, @hfinkel wrote:

> In D62199#1517071 <https://reviews.llvm.org/D62199#1517071>, @arsenm wrote:
> > In D62199#1515385 <https://reviews.llvm.org/D62199#1515385>, @ABataev wrote:
> >
> > >
> >
> >
> >
> >
> > > These patches are really NFC.  They do not add a new functionality, they simplify the existing one. And we already have the tests for these changes in NVPTX part of the libomptarget. These tests are not affected by these patches. That's why they are NFC.
> >
> > This seems like a misunderstanding of the intent of NFC to me. NFC is for commits that are not supposed to have any observable effect, like refactoring. Anything that fixes a problem, while not providing what might be considered a feature or new functionality, is not NFC
> @arsenm is certainly correct. The developer policy speaks to this as well, stating, "Avoid committing formatting- or whitespace-only changes outside of code you plan to make subsequent changes to. Also, try to separate formatting or whitespace changes from functional changes, either by correcting the format first (ideally) or afterward. Such changes should be highly localized and the commit message should clearly state that the commit is not intended to change functionality, usually by stating it is NFC." The policy does not explicitly state that this is the only valid use of NFC, and it is sometimes used for refactoring as well, but if a bug is fixed, we don't consider that to be NFC (because the effective functionality of the product has changed).

In some cases a "fix" can be NFC, if it fixes the bug in the implementation of the non-functional requirement, for example.
Functional changes - the changes that change functionality, i.e. change the expected output for the subset of inputs. Generally speaking, "volatile" qualifier does not change the logical dependency between inputs and expected outputs, it changes how the compiler handles such variables and optimizes them. But the logic remains the same.
I'm not sure that some internal compiler/programming language requirements, can be considered as functional. 
Functional requirements are supported by non-functional requirements (also known as "quality requirements"), which impose constraints on the design or implementation. In this case, "volatile" modifier can be considered as the implementation constraint.




More information about the Openmp-commits mailing list