Page MenuHomePhabricator

AMDGPU/SI: Detect dependency types between blocks
ClosedPublic

Authored by axeldavy on Feb 18 2017, 1:33 PM.

Details

Summary

Detecting the type of dependencies between blocks
enables better handling of high latencies, as
order dependencies can be differentiated from
data dependencies (in the former you don't need
to insert a wait, whereas you need in the latter).

Diff Detail

Repository
rL LLVM

Event Timeline

axeldavy created this revision.Feb 18 2017, 1:33 PM
arsenm added inline comments.Feb 21 2017, 3:24 PM
lib/Target/AMDGPU/SIMachineScheduler.cpp
541–544 ↗(On Diff #89046)

Factor into predicate function and assert on that condition

562–563 ↗(On Diff #89046)

std::make_pair

586 ↗(On Diff #89046)

Debug printing not guarded by DEBUG. Looks like this problem existed already, so this whole block should be under DEBUG

lib/Target/AMDGPU/SIMachineScheduler.h
101 ↗(On Diff #89046)

Don't need enum keyword

124 ↗(On Diff #89046)

Return ArrayRef

axeldavy added inline comments.Mar 10 2017, 1:53 PM
lib/Target/AMDGPU/SIMachineScheduler.cpp
586 ↗(On Diff #89046)

It is in a function printDebug, protected by #ifndef NDEBUG

axeldavy updated this revision to Diff 91407.Mar 10 2017, 1:56 PM
vpykhtin added inline comments.
lib/Target/AMDGPU/SIMachineScheduler.cpp
586 ↗(On Diff #89046)

Please provide full context diff next time (-U9999999), it helps to avoid such misunderstandings.

Better to use const reference to &S here.

1365 ↗(On Diff #91787)

const auto &Succ

1367 ↗(On Diff #91787)

In genereal please don't duplicate expressions. Compilers are good in CSE, but my eyes aren't :-)

This can be rewritten as Height = std::min(Height, Succ.first->Height + 1)

1650 ↗(On Diff #91787)

const auto &Block

1653 ↗(On Diff #91787)

if (--BlockNumPredsLeft[Block.first->getID()] == 0)

axeldavy updated this revision to Diff 92677.Mar 22 2017, 12:18 PM

Updated according to comments

t-tye added a subscriber: t-tye.Mar 22 2017, 6:39 PM
tony-tye removed a subscriber: tony-tye.Mar 22 2017, 6:46 PM
vpykhtin accepted this revision.Mar 23 2017, 7:46 AM

LGTM.

This revision is now accepted and ready to land.Mar 23 2017, 7:46 AM
This revision was automatically updated to reflect the committed changes.