[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