[Openmp-commits] [PATCH] D40920: Unify build documentation and convert to reStructuredText

[George Rokos via Phabricator via Openmp-commits]

grokos added inline comments.


================
Comment at: README.rst:40
+- The file CMakeCache.txt which is created after the first call to CMake is a
+  configuration file which holds all the values for the build options. These
+  configuration values can be changed using a text editor to modify
----------------
all values (remove "the")


================
Comment at: README.rst:186
+  Include adaptive locks, based on Intel(R) Transactional Synchronization
+  Extensions (Intel(R) TSX)? This is x86 specific. This feature is turned ``ON``
+  by default for IA-32 architecture and Intel(R) 64 architecture.
----------------
What is that question mark?


================
Comment at: README.rst:231
+**LIBOMP_ASMFLAGS** = <space-separated flags>
+  Additional assembly flags.
+
----------------
assembly --> assembler?


================
Comment at: README.rst:256-258
+**LIBOMPTARGET_NVPTX_ENABLE_BCLIB** = ``OFF|ON``
+  Enable CUDA LLVM bitcode offloading device RTL. This is used for link time
+  optimization of the OpenMP runtime and application code.
----------------
We had an internal discussion about it. Do we have consensus that we'll support bclib?


================
Comment at: README.rst:281
+  "30,35" should be used.
+
+Example usages of CMake
----------------
I think we should we also include something like:
```
**LIBOMPTARGET_NVPTX_DEBUG**
Enable printing of debug messages from the nvptx runtime.
```
Following you comments on the nvptx patch, I added this entry in my local branch and I was planning to include it in the next diff. You can add it in this patch instead. However, there are no ON or OFF options, the symbol is either defined or not defined.


================
Comment at: README.rst:317-318
+
+- Have CMake find the C/C++ compiler, and specify additional flags for the C
+  compiler, preprocessor, and C++ compiler.
+
----------------
Remove commas before "and" (2 places).


Repository:
  rOMP OpenMP

https://reviews.llvm.org/D40920