This patch mostly copies the existing Instruction Flow, and stage descriptions
from the mca README. I made a few text tweaks, but no semantic changes,
and made reference to the "default pipeline." I also removed the internals
references (e.g., reference to class names and header files). I did leave the
LSUnit name around, but only as an abbreviated word for the load-store unit.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
Hi Matt,
LGTM if you address the comments below.
Thanks,
-Andrea
docs/CommandGuide/llvm-mca.rst | ||
---|---|---|
579–583 ↗ | (On Diff #156863) | I don't think you should describe what a scheduling model is. We can assume that the reader knows about it. You can replace most of this paragraph with a simple sentence like: The processor Dispatch Width defaults to the value of field "IssueWidth" in the scheduling model. |
589 ↗ | (On Diff #156863) | s/pipeline/processor |
591 ↗ | (On Diff #156863) | s/temporary/physical |
595–596 ↗ | (On Diff #156863) | This sentence is not needed. It is implied by the next sentence. |
598 ↗ | (On Diff #156863) | For consistency, please replace every occurrence of temporary register with physical register. The original RFC used the word "temporary" to describe microarchitectural registers; in code comments, we always use "physical register" instead. |
609 ↗ | (On Diff #156863) | You can be more specific, and add something like "by field MicroOpBufferSize in the target scheduling model`. |
616–618 ↗ | (On Diff #156863) | This paragraph is probably not needed and can be removed. |
622 ↗ | (On Diff #156863) | Each processor scheduler implements a buffer of instructions. |
623 ↗ | (On Diff #156863) | in a scheduler's buffer |
631 ↗ | (On Diff #156863) | s/used/consumed |
655–657 ↗ | (On Diff #156863) | This paragraph is not needed. |
674–676 ↗ | (On Diff #156863) | These two sentences can be joined together. |
694–695 ↗ | (On Diff #156863) | This is no longer true. Please have a look at LSUnit.h. You will see an updated version of this paragraph (you can probably copy/paste it here). |
699–700 ↗ | (On Diff #156863) | noalias defaults to true in the current llvm-mca. |
704–705 ↗ | (On Diff #156863) | This statement should only apply to the "default" LSUnit class. |
716–719 ↗ | (On Diff #156863) | This paragraph can be removed. |
Thanks for the comments! I have updated the diff based on that feedback.
docs/CommandGuide/llvm-mca.rst | ||
---|---|---|
598 ↗ | (On Diff #156863) | I am happy to make this change to the contents of what is added by this patch. Elsewhere in the mca docs we mention 'temporary register'. Notably in mca's command line option -register-file-size. I'm happy to make changes outside of the contents of this patch in a separate patch. If we change the option documentation, I think that we will also want to change the cl::opt description in the source as well. |