This is an archive of the discontinued LLVM Phabricator instance.

Unify build documentation and convert to reStructuredText
ClosedPublic

Authored by Hahnfeld on Dec 6 2017, 1:51 PM.

Details

Summary

We now have several options that apply for both libraries and they
shouldn't be documented in multiple files. When already merging
the two Build_With_CMake.txt documents, convert them to
reStructuredText which is used for all of LLVM's documentation.

Diff Detail

Event Timeline

Hahnfeld created this revision.Dec 6 2017, 1:51 PM

This is a first version, I will do another pass tomorrow. Proof-reading would be much appreciated :-)

grokos added inline comments.Dec 6 2017, 6:26 PM
README.rst
41

all values (remove "the")

187

What is that question mark?

232

assembly --> assembler?

257–259

We had an internal discussion about it. Do we have consensus that we'll support bclib?

282

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.

318–319

Remove commas before "and" (2 places).

grokos added inline comments.Dec 6 2017, 6:41 PM
README.rst
278

The default is still sm_30, let's patch clang first to use sm_35. Also, given that the nvptx runtime cannot be built with sm<35, should we add to the description that sm_35 is the minimum required?

Hahnfeld updated this revision to Diff 125947.Dec 7 2017, 6:53 AM
Hahnfeld marked 4 inline comments as done.

Improvements based on review comments and extensions to some paragraphs.

README.rst
257–259

For now, I've only copied what has been in libomptarget/Build_With_CMake.txt. To be honest I think we should remove this paragraph entirely because the NVPTX runtime hasn't been committed yet so that we don't confuse the users.

318–319

Some of the comas are intentional to be consistent with the rest of LLVM's documentation: https://en.wikipedia.org/wiki/Serial_comma

Hahnfeld updated this revision to Diff 127852.Dec 21 2017, 3:02 AM

Removed documentation about the NVPTX runtime for now which isn't committed yet.

Ping. Is this ok to land before the 6.0 branch?

omalyshe requested changes to this revision.Dec 21 2017, 4:21 AM
omalyshe added a subscriber: omalyshe.
omalyshe added inline comments.
README.rst
142

The correct name is "Intel® Many Integrated Core Architecture (Intel® MIC Architecture)".

154

Linux*

155

Windows*
macOS*

169

macOS*
and the same in the next line OS X* ->macOS*

196

Probably it's worth to add "Include support for the OpenMP Tools Interface (OMPT)"

This revision now requires changes to proceed.Dec 21 2017, 4:21 AM
Hahnfeld updated this revision to Diff 127890.Dec 21 2017, 7:54 AM
Hahnfeld marked 5 inline comments as done.

Address review comments and pay attention to protected names.

This revision is now accepted and ready to land.Dec 21 2017, 10:30 PM
This revision was automatically updated to reflect the committed changes.
libomptarget/README.txt