Page MenuHomePhabricator

[MCA] Add an experimental MicroOpQueue stage.
ClosedPublic

Authored by andreadb on Mar 28 2019, 6:03 AM.

Details

Summary

This patch adds an experimental stage named MicroOpQueueStage.
A MicroOpQueueStage can be used to simulate a hardware micro-op queue (basically, a decoupling queue between 'decode' and 'dispatch').
Users can specify a queue size, as well as a optional MaxIPC (which - in the absence of a "Decoders" stage - can be used to simulate a different throughput from the decoders).

This stage is added to the default pipeline between the EntryStage and the DispatchStage only if PipelineOption::MicroOpQueue is different than zero. By default, llvm-mca sets PipelineOption::MicroOpQueue to the value of hidden flag -micro-op-queue-size.

Throughput from the decoder can be simulated via another hidden flag named -decoder-throughput.
That flag allows us to quickly experiment with different frontend throughputs. For targets that declare a loop buffer, flag -decoder-throughput allows users to do multiple runs, each time simulating a different throughput from the decoders.

This stage can/will be extended in future. For example, we could add a "buffer full" events to identify bottlenecks caused by backpressure. flag -decoder-throughput would probably go away if in future we delegate to another stage (DecoderStage?) the simulation of a (potentially variable) throughput from the decoders.
For now, flag -decoder-throughput is "good enough" to run some simple experiments.

Let me know if okay to commit.

-Andrea

Diff Detail

Repository
rL LLVM

Event Timeline

andreadb created this revision.Mar 28 2019, 6:03 AM

Only cosmetic comments, this looks good !

include/llvm/MCA/Context.h
42 ↗(On Diff #192615)

Maybe add a comment: // Instructions per cycle.

include/llvm/MCA/Stages/MicroOpQueueStage.h
44 ↗(On Diff #192615)

This duplicates Buffer.size(), what about removing it or making it an accessor ?

tools/llvm-mca/llvm-mca.cpp
110 ↗(On Diff #192615)

This can be understood as input (instructions) or output (uops) throughput, so I think we should clarify: Maximum throughput from the decoders (instructions per cycle)

andreadb marked 3 inline comments as done.Mar 28 2019, 7:52 AM
andreadb added inline comments.
include/llvm/MCA/Context.h
42 ↗(On Diff #192615)

Sure. I will add that comment.

include/llvm/MCA/Stages/MicroOpQueueStage.h
44 ↗(On Diff #192615)

Good point. I will remove it. I will move that code comment to method getNormalizedOpcodes.

tools/llvm-mca/llvm-mca.cpp
110 ↗(On Diff #192615)

I will change it.

andreadb updated this revision to Diff 192639.Mar 28 2019, 8:05 AM

Patch updated.

Address review comments:

  • removed field BufferSize from MicroOpQueueStage
  • added code comments
  • improved flag description in the llvm-mca driver.
RKSimon added inline comments.Mar 28 2019, 10:29 AM
lib/MCA/CMakeLists.txt
13 ↗(On Diff #192639)

Alphabetical order?

lib/MCA/Context.cpp
21 ↗(On Diff #192639)

order?

andreadb updated this revision to Diff 192680.Mar 28 2019, 10:41 AM

Address review comment.

andreadb marked 4 inline comments as done.Mar 28 2019, 10:42 AM
mattd added a comment.Mar 28 2019, 9:13 PM

LGTM!

include/llvm/MCA/Stages/MicroOpQueueStage.h
1 ↗(On Diff #192680)

nit: s/MicroOpQueue.h/MicroOpQueueStage.h/

16 ↗(On Diff #192680)

nit: Perhaps rename the guard to "LLVM_MCA_MICRO_OP_QUEUE_STAGE" . I know that looks a little nasty, but it follows the header-guard convention.

51 ↗(On Diff #192680)

Unnecessary-pedantic nit: Your std::min call at line 56 is written in a different order, with std::min(Buffer.size(), NumMicroOpcodes)

88 ↗(On Diff #192680)

nit: Copy-pasta comment. Now I know what your favorite stage is.

lib/MCA/Context.cpp
60 ↗(On Diff #192680)

Do you need a std::move here?

mattd accepted this revision.Mar 28 2019, 9:15 PM
This revision is now accepted and ready to land.Mar 28 2019, 9:15 PM
andreadb marked 5 inline comments as done.Mar 29 2019, 4:19 AM
andreadb added inline comments.
include/llvm/MCA/Stages/MicroOpQueueStage.h
1 ↗(On Diff #192680)

Thanks for catching it. I will fix it.

16 ↗(On Diff #192680)

You are absolutely right.
The original name of that stage was BufferStage. But then I opted for MicroOpQueueStage. Apparently I forgot to find&replace all the occurrences of buffer stage in this code...

51 ↗(On Diff #192680)

.. or I can just write "minimum between NumMicroOpcodes and the buffer size".

88 ↗(On Diff #192680)

hehehe. :-)

lib/MCA/Context.cpp
60 ↗(On Diff #192680)

I'll double check before committing it. Probably not...

Closed by commit rL357248: [MCA] Add an experimental MicroOpQueue stage. (authored by adibiagio, committed by ). · Explain WhyMar 29 2019, 5:15 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2019, 5:15 AM