Page MenuHomePhabricator

[MCA][Scheduler] Use latency information to further classify busy instructions.
ClosedPublic

Authored by andreadb on Feb 11 2019, 10:51 AM.

Details

Summary

This patch introduces a new instruction stage named 'IS_WAITING'.
An instruction transitions from the IS_DISPATCHED to the IS_WAITING stage if the latency of all input register operands are known.

This patch also adds a new set of instructions named 'PendingSet' to class Scheduler. The idea is that the PendingSet will only contain instructions that have reached the IS_WAITING stage.
By construction, an instruction in the PendingSet is only dependent on instructions that have already reached the execution stage. The plan is to use this knowledge to identify bottlenecks caused by data dependencies (see PR37494).

Diff Detail

Repository
rL LLVM

Event Timeline

andreadb created this revision.Feb 11 2019, 10:51 AM

LGTM. I had a few suggestions but nothing looks wrong to me.

lib/MCA/HardwareUnits/Scheduler.cpp
99 ↗(On Diff #186285)

Suggestion: if (HasDependentUsers && promoteToPendingSet()) ...

143 ↗(On Diff #186285)

Suggestion: s/RemovedElements/PromotedElements/

RKSimon added inline comments.Feb 11 2019, 11:19 AM
include/llvm/MCA/HardwareUnits/Scheduler.h
98 ↗(On Diff #186285)

If possible it'd be useful to comment these sets and explain their relationship to the Instruction::InstrStage enum values

courbet added inline comments.Feb 12 2019, 12:43 AM
lib/MCA/Instruction.cpp
161 ↗(On Diff #186285)

the capture is no longer needed.

andreadb marked 2 inline comments as done.Feb 12 2019, 3:00 AM
andreadb added inline comments.
include/llvm/MCA/HardwareUnits/Scheduler.h
98 ↗(On Diff #186285)

Sure. I will add a comment.

lib/MCA/Instruction.cpp
161 ↗(On Diff #186285)

Nice catch. I will fix it. Thanks

andreadb updated this revision to Diff 186442.Feb 12 2019, 4:51 AM
andreadb marked 4 inline comments as done.

Addressed review comments.

For consistency, I renamed instruction stage IS_WAITING to IS_PENDING.

courbet accepted this revision.Feb 12 2019, 6:16 AM
This revision is now accepted and ready to land.Feb 12 2019, 6:16 AM
mattd accepted this revision.Feb 12 2019, 9:26 AM
RKSimon accepted this revision.Feb 12 2019, 9:53 AM

LGTM with a couple of minors

include/llvm/MCA/Instruction.h
458 ↗(On Diff #186442)

IS_PENDING

lib/MCA/Instruction.cpp
163 ↗(On Diff #186442)

As discussed offline please consistently embed/separate all the lambdas and the all_of calls.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 13 2019, 3:02 AM