-
Notifications
You must be signed in to change notification settings - Fork 124
Update main.yml
workflow
#152
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
permissions: | ||
contents: write |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ensures that secrets.GITHUB_TOKEN
has permission to write to a secondary branch. This allows other people to fork mlir-www
and have a working workflow. See, for example, JuliaCI/PkgTemplates.jl#347 for more information about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add a comment for this in the file itself?
@@ -59,8 +61,9 @@ jobs: | |||
|
|||
- name: Deploy | |||
uses: peaceiris/actions-gh-pages@v3 | |||
if: github.ref == 'refs/heads/main' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This ensures that the pages are only published when a workflow is triggered by a change to main
. Copied from https://github.com/peaceiris/actions-gh-pages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, didn't do any exhaustive checking but it looked good from random sampling.
This patch fixes the equations on the Quantization page (https://mlir.llvm.org/docs/Quantization/). I don't know what caused the equations to be broken, it might be llvm/mlir-www#152, but I'm not sure. Irregardless, let's just fix it and be done with it. I've fixed the equations by moving some subscripts to the text. For some reason, the large number of subscripts caused Mathjax to fail. I've also tried KaTeX, which failed at exactly the same number of subscripts. The workflow to inspect the fix is as follows: ``` $ git clone --depth=1 https://github.com/llvm/mlir-www.git /some/path/mlir-www $ git clone --depth=1 https://github.com/llvm/llvm-project.git /some/path/llvm-project $ cp /some/path/llvm-project/mlir/docs/Quantization.md \ /some/path/mlir-www/website/content/Quantization.md $ cd /some/path/mlir-www/website $ hugo serve [...] Web Server is available at http://localhost:1313/ (bind address 127.0.0.1) Press Ctrl+C to stop ``` and view the page at http://localhost:1313/Quantization/. Reviewed By: stellaraccident Differential Revision: https://reviews.llvm.org/D152651
In response to #152 (comment).
This patch is improves upon https://reviews.llvm.org/D152621. There, I pointed out some issues with D152621, which I'll repeat here. > Passes use a different logic for generating the documentation; which I didn't update to be in-line with this change. Fixed by defining and using `mlir::tblgen::emitSummary`. This is now used in `OpDocGen.cpp` and `PassDocGen.cpp`. Note that the passes documentation currently prints the summary behind the pass argument. For example: ``` #### -arm-neon-2d-to-intr: Convert Arm NEON structured ops to intrinsics ``` at https://mlir.llvm.org/docs/Passes/#-promote-buffers-to-stack-promotes-heap-based-allocations-to-automatically-managed-stack-based-allocations. This currently differs from how the summary is printed for Ops. For example: ``` #### amdgpu.lds_barrier (::mlir::amdgpu::LDSBarrierOp) ¶ **Summary:** _Barrier that includes a wait for LDS memory operations._ ``` at https://mlir.llvm.org/docs/Dialects/AMDGPU/#amdgpulds_barrier-mliramdgpuldsbarrierop. The changes in this patch ensure that: 1. The summary is always printed on a new line. 2. The summary is always printed in italic. 3. The summary always starts with a capital letter. I've dropped the `**Summary:**`, which was introduced in D152621, because only italicization should be already clear enough. > `amx.tdpbssd` shows **Summary:** __ meaning that apparently hasSummary does not guarantee a non-empty summary. This is fixed by double-checking `!summary.empty()`, because the following code ```cpp void mlir::tblgen::emitSummary(StringRef summary, raw_ostream &os) { if (!summary.empty()) { char first = std::toupper(summary.front()); llvm::StringRef rest = summary.drop_front(); os << "\n_" << first << rest << "_\n\n"; } else { os << "\n_" << "foo" << "_\n\n"; } } ``` generates the following Markdown: ``` ### `amx.tdpbssd` (::mlir::amx::x86_amx_tdpbssd) _foo_ ``` in `tools/mlir/docs/Dialects/AMX.md`. > Summary fields containing * cancel the italicization, so the * should probably be escaped to solve this. EDIT: Nope. This is because mlir-www runs Hugo 0.80 whereas 0.111 correctly parses _Raw Buffer Floating-point Atomic Add (MI-* only)_ as an italicized string. This will be fixed by llvm/mlir-www#152. Reviewed By: jpienaar Differential Revision: https://reviews.llvm.org/D152648
This PR suggests to update Hugo to a (much) newer Hugo version. This fixes an issue where the sentence
is not parsed as an italicized sentence, see https://mlir.llvm.org/docs/Dialects/AMDGPU/#amdgpuraw_buffer_atomic_fadd-mliramdgpurawbufferatomicfaddop. This came up in https://reviews.llvm.org/D152621.
A preview of the website generated with Hugo 0.111 is available at https://rikhuijzer.github.io/mlir-www. Note that any relative link like
/foo/bar.png
is broken on that website because I'm serving the preview at/mlir-www/...
instead of/...
. Apart from those links, I am not able to find any issues related to updating the Hugo version.