[AIE2/2P] Fix DONE latency at region end. #345
+62
−0
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
SagarMaheshwari99
requested review from
abhinay-anubola,
abnikant,
andcarminati,
F-Stuckmann,
gbossu,
katerynamuts,
khallouh,
konstantinschwarz,
martien-de-jong,
niwinanto and
stephenneuendorffer
as code owners
| @@ -176,6 +176,13 @@ struct AIEBaseInstrInfo : public TargetInstrInfo { | |||
| } | |||
| /// Check whether Opc represents a lock instruction | |||
| virtual bool isLock(unsigned Opc) const { return false; } | |||
|
|
|||
| /// Check whether Opc represents a DONE instruction | |||
| virtual bool isDone(unsigned Opc) const { return false; } | |||
There was a problem hiding this comment.
nit: I hope no one needs to call this from outside. This one could go and instances could be private or inlined.
martien-de-jong
approved these changes
Feb 10, 2025
gbossu
reviewed
Feb 10, 2025
| virtual bool isDone(unsigned Opc) const { return false; } | ||
|
|
||
| /// Get "implicit" latency for special instructions. | ||
| virtual unsigned getImplicitLatency(const MachineInstr &) const { return 0; } |
There was a problem hiding this comment.
Can you document what that means?
gbossu
reviewed
Feb 10, 2025
| @@ -280,6 +280,10 @@ class RegionEndEdges : public ScheduleDAGMutation { | |||
| EdgeLatency = DelaySlots + 1; | |||
| } | |||
|
|
|||
| // "Implicit" latency for special instructions. | |||
| const unsigned ImplicitLatency = TII->getImplicitLatency(MI); | |||
| EdgeLatency = std::max(EdgeLatency, ImplicitLatency); | |||
There was a problem hiding this comment.
FYI @mludevid you might have done something similar when working on semaphores.
| @@ -176,6 +176,13 @@ struct AIEBaseInstrInfo : public TargetInstrInfo { | |||
| } | |||
| /// Check whether Opc represents a lock instruction | |||
| virtual bool isLock(unsigned Opc) const { return false; } | |||
|
|
|||
| /// Check whether Opc represents a DONE instruction | |||
| virtual bool isDone(unsigned Opc) const { return false; } | |||
There was a problem hiding this comment.
I think we can have a design where we need just one method to be reimplemented. Something like:
virtual std::optional<unsigned> getDoneLatency(...)
In this case, we can have a base implementation for getImplicitLatency that can be reimplemented by the target only if needed.
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.