This is an archive of the discontinued LLVM Phabricator instance.

[docs] Update README and GettingStarted
ClosedPublic

Authored by aeubanks on Mar 6 2023, 11:48 AM.

Details

Summary

Funnel fetching and building LLVM instructions into GettingStarted.

Modernize the build steps a little.

Remove comments saying CMAKE_BUILD_TYPE defaults to Debug as that's not true anymore (must explicitly pass it).

Diff Detail

Event Timeline

aeubanks created this revision.Mar 6 2023, 11:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 6 2023, 11:48 AM
Herald added a subscriber: dschuff. · View Herald Transcript
aeubanks requested review of this revision.Mar 6 2023, 11:48 AM
Herald added a project: Restricted Project. · View Herald Transcript
MaskRay accepted this revision.Mar 6 2023, 1:07 PM

LGTM. Deduplicating README.md makes sense and removes one place to change when we update instructions.
Changes like cmake -S LG, too.

llvm/docs/GettingStarted.rst
30

Check out

This revision is now accepted and ready to land.Mar 6 2023, 1:07 PM
MaskRay added inline comments.Mar 6 2023, 1:10 PM
llvm/docs/GettingStarted.rst
70

Keep double backquotes for Debug, Release, ...

95–105

... building just llvm/, not other subprojects

MaskRay added inline comments.Mar 6 2023, 1:11 PM
llvm/docs/GettingStarted.rst
607–611

LLVM_TARGETS_TO_BUILD=host is supported and seems preferred to include just the host architecture.

aeubanks updated this revision to Diff 502780.Mar 6 2023, 1:33 PM

address comments
also remove comment saying CMAKE_BUILD_TYPE defaults to Debug as that's not true anymore

aeubanks edited the summary of this revision. (Show Details)Mar 6 2023, 1:33 PM
aeubanks marked 4 inline comments as done.
aeubanks added inline comments.
llvm/docs/GettingStarted.rst
607–611

TIL, thanks!

MaskRay accepted this revision.Mar 6 2023, 1:40 PM

Best to wait for a second opinion.

llvm/docs/GettingStarted.rst
98

Are the two code blocks look good when you read them in a browser? ninja docs-llvm-html

aeubanks marked an inline comment as done.Mar 6 2023, 1:42 PM
aeubanks added inline comments.
llvm/docs/GettingStarted.rst
98

yup, I had checked that way

Seems fine to land if nobody else has an opinion in one or two days.

hans accepted this revision.Mar 8 2023, 1:53 AM

Looks great, thanks for updating!

LGTM as-is, but I did have some comments you can feel free to take or leave.

llvm/docs/GettingStarted.rst
48

Should we link to CMake's generator documentation here?

75–76

Should we advertise LLVM_PARALLEL_{COMPILE,LINK}_JOBS and LLVM_USE_LINKER given how important those are to getting a working build on some platforms? (This can be done later if you don't want to do it as part of this patch.)

aeubanks updated this revision to Diff 503477.Mar 8 2023, 12:55 PM
aeubanks marked an inline comment as done.

link to cmake generator docs

llvm/docs/GettingStarted.rst
75–76

will do that in a followup change

This revision was landed with ongoing or failed builds.Mar 8 2023, 12:56 PM
This revision was automatically updated to reflect the committed changes.