This is an archive of the discontinued LLVM Phabricator instance.

[llvm-mca][docs] Add instruction flow documentation. NFC.
ClosedPublic

Authored by mattd on Jul 23 2018, 1:30 PM.

Details

Summary

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.

Diff Detail

Event Timeline

mattd created this revision.Jul 23 2018, 1:30 PM
andreadb accepted this revision.Jul 30 2018, 3:49 AM

Hi Matt,

LGTM if you address the comments below.

Thanks,
-Andrea

docs/CommandGuide/llvm-mca.rst
579–583

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

s/pipeline/processor

591

s/temporary/physical

595–596

This sentence is not needed. It is implied by the next sentence.

598

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

You can be more specific, and add something like "by field MicroOpBufferSize in the target scheduling model`.

616–618

This paragraph is probably not needed and can be removed.

622

Each processor scheduler implements a buffer of instructions.

623

in a scheduler's buffer

631

s/used/consumed

655–657

This paragraph is not needed.

674–676

These two sentences can be joined together.
MCA utilizes a simulated load/store unit (LSUnit) to track execution of loads and stores

694–695

This is no longer true.
By default, the LSUnit optimistically assumes that loads don't alias store operations.

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

noalias defaults to true in the current llvm-mca.

704–705

This statement should only apply to the "default" LSUnit class.
In future, we could (should) allow users to customzie the LSUnit. That would enable the support for different memory types (and therefore, different consistency models).

716–719

This paragraph can be removed.

This revision is now accepted and ready to land.Jul 30 2018, 3:49 AM
mattd updated this revision to Diff 158077.Jul 30 2018, 2:41 PM
mattd marked 16 inline comments as done.

Thanks for the comments! I have updated the diff based on that feedback.

docs/CommandGuide/llvm-mca.rst
598

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.

This revision was automatically updated to reflect the committed changes.