[AIE NFC] Postpipeliner cleanups and refactorings #253
Conversation
martien-de-jong
requested review from
abhinay-anubola,
abnikant,
andcarminati,
gbossu,
khallouh,
konstantinschwarz,
SagarMaheshwari99 and
stephenneuendorffer
as code owners
martien-de-jong
force-pushed
the
martien.smallfixes
branch
from
2f11c48 to
3e5d4c7
Compare
| static cl::opt<int> PostPipelinerMaxTryII( | ||
| "aie-postpipeliner-maxtry-ii", cl::init(10), | ||
| cl::desc("[AIE] Maximum II steps to be tried in the post-ra pipeliner")); | ||
|
|
There was a problem hiding this comment.
Rationale: It is difficult to give a useful absolute value. for small ResMII we will be trying way beyond usefulness, for larger ResMII it may not be enough. A relative amount works better.
| @@ -600,6 +604,7 @@ SchedulingStage InterBlockScheduling::updateScheduling(BlockState &BS) { | |||
| auto &PostSWP = BS.getPostSWP(); | |||
| if (PostSWP.canAccept(*BS.TheBlock)) { | |||
| BS.FixPoint.II = PostSWP.getResMII(*BS.TheBlock); | |||
| BS.FixPoint.IITries = 1; | |||
There was a problem hiding this comment.
This also simplifies using a solver only on the first few tries
| @@ -139,9 +161,10 @@ class PostPipeliner { | |||
| int FirstUnscheduled = 0; | |||
| int LastUnscheduled = -1; | |||
|
|
|||
| /// Holds the cycle of each SUnit. The following should hold: | |||
| /// Holds the schuling information for each instruction. The following | |||
There was a problem hiding this comment.
schuling -> scheduling
| public: | ||
| std::vector<NodeInfo> Nodes; | ||
| int NInstr; | ||
| int MaxEarliest = 0; |
There was a problem hiding this comment.
It would be nice to have some comments on these fields, for future reference.
There was a problem hiding this comment.
Check: those fields are not used now. Are they related to a new feature?
There was a problem hiding this comment.
ScheduleInfo is passed to the strategies. These values are computed as a service to implementors of new strategies.
| # See https://llvm.org/LICENSE.txt for license information. | ||
| # SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
| # | ||
| # (c) Copyright 2024 Advanced Micro Devices, Inc. or its affiliates |
| # | ||
| # (c) Copyright 2024 Advanced Micro Devices, Inc. or its affiliates | ||
|
|
||
| # RUN: llc --mtriple=aie2 -O2 --start-before=postmisched %s \ |
There was a problem hiding this comment.
Can we have a description of what does feasibleRA means here? I can see that we have some conf2d_bf16 so it is interesting to also see how this one differs from the others.
|
Hi @martien-de-jong, this set of changes looks good! I just left some comments for clarification! |
This is to give full access to the Info array and it's associated parameters
Dump intervals in ascii art
martien-de-jong
force-pushed
the
martien.smallfixes
branch
from
3e5d4c7 to
95dbddf
Compare
Small logging improvements
Facility to rerun a strategy an arbitrary number of times
Push earliest with 'local' resource conflicts