Page MenuHomePhabricator

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

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



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


Event Timeline

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

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

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
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
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.
98 ↗(On Diff #186285)

Sure. I will add a comment.

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

458 ↗(On Diff #186442)


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