[PATCH] D122990: Added new file & improved inclusivity
Aaron Ballman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 4 11:40:33 PDT 2022
aaron.ballman added inline comments.
================
Comment at: README.md:36-37
-The LLVM Getting Started documentation may be out of date. The [Clang
-Getting Started](http://clang.llvm.org/get_started.html) page might have more
+The LLVM Getting Started documentation may be out of date. The Clang
+[Getting Started](http://clang.llvm.org/get_started.html) page might have more
accurate information.
----------------
Removing the extra whitespace is fine, but I don't understand why you removed `Clang` from the link title. (I think it's more clear with the `Clang` because it distinguishes it from the LLVM getting started page but maybe you've got a different idea?)
================
Comment at: mlir/README.md:34
+
+**See [here](https://mlir.llvm.org/) for more information.**
+
----------------
mehdi_amini wrote:
> I rather not duplicate the website here, why not just pointing there?
+1, we should just be able to point the user to the MLIR website, I think.
================
Comment at: mlir/docs/LangRef.md:23
transformations and analysis, and a compact serialized form suitable for storage
-and transport. The different forms all describe the same semantic content. This
+and transport. All the different forms describes the same semantic content. This
document describes the human-readable textual form.
----------------
I don't think this edit is necessary, but if you wanted to reword this way, it should be "All the different forms describe the same semantic content." to be grammatically correct.
================
Comment at: mlir/docs/OpDefinitions.md:214-215
+All the arguments should be named to:
+- provide documentation.
+- drive auto-generation of getter methods.
+- provide a handle to reference for other places like constraints.
----------------
================
Comment at: mlir/docs/OpDefinitions.md:438
declaration. This works for `BoolAttr`, `StrAttr`, `EnumAttr` for now and the
-list can grow in the future. So if possible, default valued attribute should be
+list can grow in the future. So if possible, the default valued attribute should be
placed at the end of the `arguments` list to leverage this feature. (This
----------------
================
Comment at: mlir/docs/OpDefinitions.md:584-597
1. StructuralOpTrait will be verified first, they can be run independently.
1. `verifyInvariants` which is constructed by ODS, it verifies the type,
attributes, .etc.
1. Other Traits/Interfaces that have marked their verifier as `verifyTrait` or
`verifyWithRegions=0`.
-1. Custom verifier which is defined in the op and has marked `hasVerifier=1`
+1. Custom verifier which is defined in the op and has been marked `hasVerifier=1`
----------------
It looks like the numbered lists here are always `1.` and that should probably be fixed up.
================
Comment at: mlir/docs/README.md:1-8
+# MLIR documentation
+
+Please note [this](mlir.llvm.org) is where MLIR's rendered documentation is displayed.
+The viewing experience on GitHub or elsewhere may not match those of the
+website. For any changes please verify instead that they work on the main
+website first.
+
----------------
Why is this file necessary? There's already a README.txt file, and none of the other top-level projects have a README.md
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122990/new/
https://reviews.llvm.org/D122990
More information about the llvm-commits
mailing list