[AIE][PostPipeliner] make PostPipeliner depend less on PostScheduler #740
Conversation
martien-de-jong
requested review from
F-Stuckmann,
SagarMaheshwari99,
abhinay-anubola,
abnikant,
andcarminati,
katerynamuts,
khallouh,
konstantinschwarz,
mludevid,
niwinanto and
stephenneuendorffer
as code owners
| // We virtually unroll the body by the stagecount, computed from rounding | ||
| // up the length divided by II, adding one more stage to account for | ||
| // the added resource contention | ||
| NCopies = (BS.getScheduleLength() + II - 1) / II + 1; |
There was a problem hiding this comment.
This was the last of the remaining dependences that required the scheduler to run before the pipeliner
| LLVM_DEBUG(dbgs() << "Conflict in iteration N=" << N << "\n"); | ||
| int Iter = 0; | ||
| while (Cycle < Horizon) { | ||
| if (HR.checkConflict(Scoreboard, *MI, Cycle)) { |
There was a problem hiding this comment.
This scoreboard is no longer used for the correctness check, which we now do explicitly in scheduleOtherIterations. Here we just foreshadow conflicts with the first iteration.
| // pushed by LCDs from the first iteration. Since the dag doesn't change, | ||
| // the third iteration behaves the same. | ||
| for (int K = 0; K < NInstr; K++) { | ||
| const int N = K + NInstr; |
There was a problem hiding this comment.
we lost an outer loop, so we have a major diff on indentation. NInstr corresponds to the former outerloop control variable L.
| DEBUG_SUMMARY(dbgs() << "PostPipeliner: Unsafe stage count, NCopies=" | ||
| << NCopies << "\n"); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
We can't makes this check anymore, but we also 'reach steady state' by constructing it directly from NStages
martien-de-jong
force-pushed
the
martien.aiecc-1093
branch
2 times, most recently
from
739671d to
86b1250
Compare
|
|
||
| // Initialize slot counts. | ||
| for (int K = 0; K < NTotalInstrs; K++) { | ||
| for (int K = 0; K < NInstr; K++) { |
There was a problem hiding this comment.
Could we clarify if we are using NInstr of ScheduleInfo or of PostPipeliner ?
Are they the same? It seems as if PostPipeliner::NInstr is closer to ScheduleInfo::Nodes
There was a problem hiding this comment.
Good point. NInstr was added to ScheduleInfo at some point; I guess the PostPipeliner one is redundant and should be eliminated. But whenever you see NInstr anywhere in postpipeliner code, it is the number schedulable instructions in the basic block, i.e. the number of nodes in one copy of the dag.
There was a problem hiding this comment.
On second thought, no. Ninstr is copied into ScheduleInfo to decouple an interface from the PostPipeliner itself, but most of the NInstr occurrences are to that of the PostPipeliner. Anyway, they both represent the same invariant number.
|
|
||
| LLVM_DEBUG(dbgs() << "Final Earliest - Latest:\n"); | ||
| for (int K = 0; K < NTotalInstrs; K++) { | ||
| for (int K = 0; K < 2 * NInstr; K++) { |
There was a problem hiding this comment.
nit: for consistency can we use int(Info.Nodes.size()) like above?
There was a problem hiding this comment.
or better, run a range based for loop.
AIECC-1093 This makes the postpipeliner more flexible. We no longer need to set the number of copies predicting the stage count. DAG and SchedInfo nodes are fixed at two copies. The scoreboard is reduced to the lookahead the pipeliner needs to see the future conflicts with instructions scheduled earlier, which is a pure heuristic. Correctness is checked separately by populating a scoreboard with the steady state. This check can advance the scoreboard, since it only needs to check the loop body stage. Hence it can be of fixed size II + pipelinedepth. The change also reduces runtime, especially for DAG construction
martien-de-jong
force-pushed
the
martien.aiecc-1093
branch
from
86b1250 to
a29794e
Compare
AIECC-1093
This makes the postpipeliner more flexible.
We no longer need to set the number of copies predicting the stage count. DAG and SchedInfo nodes are fixed at two copies.
The scoreboard is reduced to the lookahead the pipeliner needs to see the future conflicts with instructions scheduled earlier, which is a pure heuristic.
Correctness is checked separately by populating a scoreboard with the steady state. This check can advance the scoreboard, since it only needs to check the loop body stage. Hence it can be of fixed size II + pipelinedepth.
The change also reduces runtime, especially for DAG construction