[PATCH] D143587: [Docs] Multilib design

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa5aeba737694: [Docs] Multilib design (authored by michaelplatings). Changed prior to commit:

[PATCH] D143587: [Docs] Multilib design

phosek accepted this revision. phosek added a comment. LGTM Comment at: clang/docs/Multilib.rst:231 + +However, an exception is the normalization of -march. +-march for Arm architectures contains a list of enabled and disabled extensions Use backticks to

[PATCH] D143587: [Docs] Multilib design

michaelplatings updated this revision to Diff 529588. michaelplatings added a comment. FlagMap->Mappings Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143587/new/ https://reviews.llvm.org/D143587 Files: clang/docs/Multilib.rst

[PATCH] D143587: [Docs] Multilib design

michaelplatings updated this revision to Diff 529569. michaelplatings added a comment. Update docs to reflect recent changes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143587/new/ https://reviews.llvm.org/D143587 Files:

[PATCH] D143587: [Docs] Multilib design

michaelplatings updated this revision to Diff 519004. michaelplatings added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143587/new/ https://reviews.llvm.org/D143587 Files: clang/docs/Multilib.rst clang/docs/index.rst

[PATCH] D143587: [Docs] Multilib design

michaelplatings updated this revision to Diff 508041. michaelplatings added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143587/new/ https://reviews.llvm.org/D143587 Files: clang/docs/Multilib.rst clang/docs/index.rst

[PATCH] D143587: [Docs] Multilib design

michaelplatings updated this revision to Diff 505451. michaelplatings marked an inline comment as done. michaelplatings added a comment. Add quotes Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143587/new/ https://reviews.llvm.org/D143587 Files:

[PATCH] D143587: [Docs] Multilib design

michaelplatings updated this revision to Diff 505293. michaelplatings added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143587/new/ https://reviews.llvm.org/D143587 Files: clang/docs/Multilib.rst clang/docs/index.rst

[PATCH] D143587: [Docs] Multilib design

MaskRay accepted this revision. MaskRay added inline comments. Comment at: clang/docs/Multilib.rst:241 + +multilib.yaml and -print-multi-selection-flags-experimental are new interfaces +to Clang. In order for them to be usable over time and across LLVM versions

[PATCH] D143587: [Docs] Multilib design

michaelplatings updated this revision to Diff 505053. michaelplatings added a comment. tags->flags Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143587/new/ https://reviews.llvm.org/D143587 Files: clang/docs/Multilib.rst clang/docs/index.rst

[PATCH] D143587: [Docs] Multilib design

michaelplatings updated this revision to Diff 504775. michaelplatings added a comment. Reference Multilib.rst from comment in Multilib.h Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143587/new/ https://reviews.llvm.org/D143587 Files:

[PATCH] D143587: [Docs] Multilib design

peter.smith accepted this revision. peter.smith added a comment. This revision is now accepted and ready to land. I've set approved from the Arm side. Please leave some time for people in the US time zone to leave any final comments or ask for extensions. I've left a suggestion that can be

[PATCH] D143587: [Docs] Multilib design

michaelplatings updated this revision to Diff 503906. michaelplatings added a comment. flags -> tags Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143587/new/ https://reviews.llvm.org/D143587 Files: clang/docs/Multilib.rst

[PATCH] D143587: [Docs] Multilib design

michaelplatings updated this revision to Diff 503046. michaelplatings added a comment. - Make "experimental" more obvious. - Demonstrate using a Clang-generated flag directly in Flags Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143587/new/

[PATCH] D143587: [Docs] Multilib design

michaelplatings updated this revision to Diff 499963. michaelplatings added a comment. PrintArgs -> PrintOptions Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143587/new/ https://reviews.llvm.org/D143587 Files: clang/docs/Multilib.rst

[PATCH] D143587: [Docs] Multilib design

michaelplatings added inline comments. Comment at: clang/docs/Multilib.rst:237 + +Stable API +-- MaskRay wrote: > What does "API" refer to? A function defined in llvm-project/clang/lib? Changed "API" to "interface" and reworded to hopefully make it

[PATCH] D143587: [Docs] Multilib design

michaelplatings updated this revision to Diff 499511. michaelplatings marked 8 inline comments as done. michaelplatings added a comment. Rebase and apply changes requested by @MaskRay and @peter.smith Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D143587: [Docs] Multilib design

MaskRay added inline comments. Comment at: clang/docs/Multilib.rst:41 + +The available libraries can be hard-coded in clang. Typically this is done +using the ``MultilibBuilder`` interface. There are many examples of this in `s/clang/Clang/`

[PATCH] D143587: [Docs] Multilib design

michaelplatings updated this revision to Diff 496533. michaelplatings added a comment. Tweak multilib.yaml spec slightly Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143587/new/ https://reviews.llvm.org/D143587 Files: clang/docs/Multilib.rst

[PATCH] D143587: [Docs] Multilib design

peter.smith added inline comments. Comment at: clang/docs/Multilib.rst:125 +``-fno-exceptions`` multilib variant need only contain C++ libraries. + +Stability Although implicit in the mechanism, is it worth highlighting that layered multib.yaml authors will

[PATCH] D143587: [Docs] Multilib design

peter.smith added a comment. Thanks for the update, one nit in the language, otherwise looks good to me. It is a good reflection of the implementation. Comment at: clang/docs/Multilib.rst:250 + +The API need only multilib selection based on only a limited set of command +line

[PATCH] D143587: [Docs] Multilib design

michaelplatings added a comment. > Could be worth adding a design principles section at the end That was a very helpful suggestion, thank you. Writing them out helped me examine the compatibility story more carefully. Consequently I added a requirement for a "minimumClangVersion" property,

[PATCH] D143587: [Docs] Multilib design

michaelplatings updated this revision to Diff 496415. michaelplatings marked 7 inline comments as done. michaelplatings added a comment. Incorporate suggestions from @peter.smith Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143587/new/

[PATCH] D143587: [Docs] Multilib design

peter.smith added a comment. Thanks very much for writing this. Will be worth updating the RFC with a link as I think that this is an approachable place to comment on the format and selection model without the implementation detail. I'm happy with what has been written so far. My suggestions

[PATCH] D143587: [Docs] Multilib design

michaelplatings marked 3 inline comments as done. michaelplatings added inline comments. Comment at: clang/docs/Multilib.rst:86-89 + a match. + If more than one variant matches then a toolchain may opt to either use only + the *last* matching multilib variant, or may use

[PATCH] D143587: [Docs] Multilib design

michaelplatings updated this revision to Diff 496072. michaelplatings added a comment. Made changes requested by @amilendra Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143587/new/ https://reviews.llvm.org/D143587 Files:

[PATCH] D143587: [Docs] Multilib design

amilendra added inline comments. Comment at: clang/docs/Multilib.rst:89 + the *last* matching multilib variant, or may use all matching variants, + thereby layering them. +#. Generate ``-isystem`` and ``-L`` arguments. Iterate in reverse order over What

[PATCH] D143587: [Docs] Multilib design

amilendra added a comment. Thanks for the design docs. At a high-level this gives a good summary on what you intend the multilib feature to do. Couple of suggestions. Comment at: clang/docs/Multilib.rst:86-89 + a match. + If more than one variant matches then a toolchain

[PATCH] D143587: [Docs] Multilib design

michaelplatings created this revision. michaelplatings added reviewers: peter.smith, phosek. Herald added a subscriber: arphaman. Herald added a project: All. michaelplatings requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG