This is an archive of the discontinued LLVM Phabricator instance.

[llvm-mca] Add flag -instruction-tables to print the theoretical resource pressure distribution for instructions (PR36874)
ClosedPublic

Authored by andreadb on Mar 23 2018, 11:41 AM.

Details

Summary

The goal of this patch is to address most of PR36874.
To fully fix PR36874 we need to split the "InstructionInfo" view from the "SummaryView". That would make easy to check the latency and rthroughput as well.

The patch reuses all the logic from ResourcePressureView to print out the "instruction tables".

We have an entry for every instruction in the input sequence. Each entry reports the theoretical resource pressure distribution. Resource pressure is uniformly distributed across all the processor resource units of a group.

At the moment, the backend pipeline is not configurable, so the only way to fix this is by creating a different driver that simply sends instruction events to the resource pressure view.
That means, we don't use the Backend interface. Instead, it is simpler to just have a different code-path for when flag -instruction-tables is specified.

Once Clement addresses bug 36663, then we can port the "instruction tables" logic into a stage of our configurable pipeline.

Updated the BtVer2 test cases (thanks Simon for the help). Now we pass flag -instruction-tables to each modified test.

Please let me know if okay to commit.

Thanks,
Andrea

Diff Detail

Event Timeline

andreadb created this revision.Mar 23 2018, 11:41 AM
andreadb updated this revision to Diff 139631.Mar 23 2018, 11:58 AM

Patch updated.

Fixed the 80-col violation in the .rst file.
Updated test resource-sse2.s.

I'm happy with this but that's because I asked for it.....

@craig.topper @courbet I'm guessing this will be alright for Intel targets as well when they get resource tests added - I don't think we've missed anything?

tools/llvm-mca/InstructionTables.cpp
40

Better to test for Resource.second.empty() and create Cycles after the continue? Comparing floats with zero always makes me nervouse - even for trivial cases like this.

tools/llvm-mca/llvm-mca.cpp
363

Commit this clang-format fix separately.

Thanks Simon

tools/llvm-mca/InstructionTables.cpp
40

Right.. I will fix it.

tools/llvm-mca/llvm-mca.cpp
363

Okay.

andreadb updated this revision to Diff 139715.Mar 24 2018, 9:15 AM

Patch updated.

Addressed Simon's review comments.

courbet accepted this revision.Mar 26 2018, 4:42 AM
courbet added inline comments.
tools/llvm-mca/InstrBuilder.h
55

I don't remember whether I saw a comment with the definition of the masks somewhere or if we just discussed that together. Can you add a comment here, either pointing to where the notion of mask is defined, or the definition (something like "ProcResourceMasks[I] & J is true if resource I has a subresource J".

tools/llvm-mca/InstructionTables.cpp
61

Getting rid of E1 and using NumUnits instead would make everything clearer.

tools/llvm-mca/InstructionTables.h
12

Please add a comment to define what "instruction tables" are, maybe just:
"see the -instruction-tables command-line flag in docs/CommandGuide/llvm-mca.rst"

This revision is now accepted and ready to land.Mar 26 2018, 4:42 AM
andreadb marked 2 inline comments as done.Mar 26 2018, 4:57 AM

Thanks Clement,

I am going to address all your review comments before committing the patch.

Cheers,
Andrea

tools/llvm-mca/InstrBuilder.h
55

These are the processor resource masks computed by function mca::computeProcResourceMasks in Support.h.
In Support.h there is a description of how the mask works.
I will add a comment explaining this.

tools/llvm-mca/InstructionTables.cpp
61

I will do it.

tools/llvm-mca/InstructionTables.h
12

Right. I will add that reference.

This revision was automatically updated to reflect the committed changes.