[AIEX] Ignore bank conflict if in the next cycle we cannot schedule the instruction. #229
+150
−17
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
andcarminati
reviewed
Nov 5, 2024
| @@ -479,6 +483,56 @@ int AIEPostRASchedStrategy::getMaxDeltaCycles(const SchedBoundary &Zone) const { | |||
| BottomUpDelta.getValue()}); | |||
| } | |||
|
|
|||
| bool AIEPostRASchedStrategy::canOptimizeMemoryAccess(SUnit &SU, | |||
There was a problem hiding this comment.
As optimize is a general term, is it possible to summarize the goal of this function? I see the explanation deep in the code, but I think that is also good to have information in advance (mostly for future reference).
andcarminati
reviewed
Nov 5, 2024
| @@ -88,6 +88,10 @@ static cl::opt<bool> UseLoopHeuristics( | |||
| "aie-loop-sched-heuristics", cl::init(true), | |||
| cl::desc("Use special picking heuristics when scheduling a loop region")); | |||
|
|
|||
| static cl::opt<bool> IgnoreMemoryBankConflict( | |||
| "aie-ignore-bank-conflict", cl::init(false), | |||
| cl::desc("Ignore bank conflicts based on special heuristics")); | |||
There was a problem hiding this comment.
nit: I like this special heuristics but I think we can go with just heuristics ;-).
andcarminati
reviewed
Nov 5, 2024
|
As an insight about the regressions, are the worst ones related to bank conflicts (wait cycles) or just longer schedules? Or even a mix? |
|
About QoR, I can see that some results are not present (like Reduce* and Softmax*). Do you know the reason? |
|
As an insight about the regressions, are the worst ones related to bank conflicts (wait cycles) or just longer schedules? Or even a mix? I have been pondering on how we can automate turning on/off this feature without been too expensive About QoR, I can see that some results are not present (like Reduce* and Softmax*). Do you know the reason? |
|
I have a feeling that, before committing to a bank conflict, we should use a finer-grained heuristic to check if we reduce resource conflicts. We can get an inspiration on @martien-de-jong's work in |
| AIEHazardRecognizer &HR = *getAIEHazardRecognizer(Zone); | ||
| MachineInstr *MI = SU.getInstr(); | ||
|
|
||
| const std::vector<unsigned int> *AlternateOpcodes; |
There was a problem hiding this comment.
I would suggest to declare and initialize it together, two lines down
Will look into it, I think we need to add an extra pass iteration of scheduler to see if we are able to reduce the number of VLIW instruction in a BB by the same or more number of times that of we are creating bank-conflict. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.