-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[AIEX] Reschedule multi-slot instruction for better packing/schedule #217
base: aie-public
Are you sure you want to change the base?
Conversation
|
Add following after #221 is merged
|
a0a8fc0
to
23f63de
Compare
23f63de
to
7398ab2
Compare
// general check for isAvailabeNode() this means we have not set the | ||
// selected opcode for the instruction. Set the selected opcode for | ||
// the instruction. | ||
if (AlternateOpcodes->size() > 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have AlternateOpcodes
and AltOpcodes
that refer to different instructions. Maybe a more comprehensive name will help to follow the code.
@@ -37,6 +37,7 @@ std::vector<AIE::MachineBundle> computeAndFinalizeBundles(SchedBoundary &Zone); | |||
class AIEPostRASchedStrategy : public PostGenericScheduler { | |||
/// Maintain the state of interblock/loop-aware scheduling | |||
AIE::InterBlockScheduling InterBlock; | |||
MutateInstructionMap MutateInstruction; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I understood, we use this map to store the information of an instruction B that should be allocated to a different slot to give a position to an instruction A (named new instruction). As we process one new instruction/time, we will use at least one position of this map. I think we can simplify this. What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From a higher perspective i.e from SchedBoundary::releasePending()
or from *SchedBoundary::pickOnlyChoice()
we we call isAvailableNode
in a loop. So we might end up having multiple entries in MutateInstructionMap
Hi @krishnamtibrewala, nice work! I left some comments, more in the direction of some simplification, before a next round of review. |
e849513
to
b4b3127
Compare
Thanks for the review @andcarminati, have updated the codebase as suggested. I look forward to next round of review. |
b4b3127
to
b136b22
Compare
Format = 0b001, | ||
MemoryBank = 0b010, | ||
FuncUnit = 0b100, | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: To avoid the constant static_cast
, I think we could create e.g.
class ConflictTypeSet {
uint32_t BitSet = 0b0;
static uint32_t asBit(ConflictType CT) { return uint32_t(CT); }
public:
bool has(ConflictType CT) const { return BitSet & asBit(CT); }
bool any() const { return BitSet != 0b0; }
void set(ConflictType CT) { BitSet |= asBit(CT); }
};
No description provided.