Skip to content
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] Ignore bank conflict if in the next cycle we cannot schedule the instruction. #229

Draft
wants to merge 2 commits into
base: aie-public
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 56 additions & 1 deletion llvm/lib/Target/AIE/AIEMachineScheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Copy link
Collaborator

@andcarminati andcarminati Nov 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I like this special heuristics but I think we can go with just heuristics ;-).


namespace {
// A sentinel value to represent an unknown SUnit.
const constexpr unsigned UnknownSUNum = ~0;
Expand Down Expand Up @@ -479,6 +483,56 @@ int AIEPostRASchedStrategy::getMaxDeltaCycles(const SchedBoundary &Zone) const {
BottomUpDelta.getValue()});
}

bool AIEPostRASchedStrategy::canOptimizeMemoryAccess(SUnit &SU,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

SchedBoundary &Zone,
const int DeltaCycle) {
if (!IgnoreMemoryBankConflict)
return false;

if (!SU.getInstr()->mayLoadOrStore()) {
return false;
}

const int MinDelta = -getMaxDeltaCycles(Zone);
if (!(DeltaCycle - 1 >= MinDelta))
return false;

const AIEBaseMCFormats &Formats = *getTII(*Zone.DAG)->getFormatInterface();
AIEHazardRecognizer &HR = *getAIEHazardRecognizer(Zone);
MachineInstr *MI = SU.getInstr();

const std::vector<unsigned int> *AlternateOpcodes;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest to declare and initialize it together, two lines down

auto DefaultOpcode = std::vector<unsigned int>{SU.getInstr()->getOpcode()};
AlternateOpcodes =
Formats.getAlternateInstsOpcode(SU.getInstr()->getOpcode())
? Formats.getAlternateInstsOpcode(SU.getInstr()->getOpcode())
: &DefaultOpcode;

unsigned int OpcodeWithMemoryBankConflict = 0;
for (const unsigned int AltOpcode : *AlternateOpcodes) {
// Check if the conflict was caused by a memory bank.
if (HR.checkConflict(*MI, getTII(*Zone.DAG)->get(AltOpcode), DeltaCycle) ==
AIEHazardRecognizer::ConflictType::MemoryBank) {
OpcodeWithMemoryBankConflict = AltOpcode;
break;
}
}
// Check if the memory operation will also have a conflict in the next cycle.
krishnamtibrewala marked this conversation as resolved.
Show resolved Hide resolved
// If so, we could schedule the instruction in the current delta cycle, even
// though it causes bank conflict.
// NOTE : With this optimization if the resultant schedule does not decrease
// the total instr. count in the kernel loop by the same number of bank
// conflict we are allowing we will see regression.
if (OpcodeWithMemoryBankConflict && Zone.checkHazard(&SU, DeltaCycle - 1)) {
if (AlternateOpcodes->size() > 1)
HR.getSelectedAltDescs().setAlternateDescriptor(
MI, OpcodeWithMemoryBankConflict);
return true;
}

return false;
}

bool AIEPostRASchedStrategy::isAvailableNode(SUnit &SU, SchedBoundary &Zone,
bool /*VerifyReadyCycle*/) {
// Whether or not the zone is Top or Bot, verify if SU is ready to be
Expand All @@ -497,7 +551,8 @@ bool AIEPostRASchedStrategy::isAvailableNode(SUnit &SU, SchedBoundary &Zone,
// ReadyCycle is always greater or equal to the current cycle,
// so DeltaCycles will always be less or equal to 0.
if (Zone.checkHazard(&SU, DeltaCycles))
continue;
if (!canOptimizeMemoryAccess(SU, Zone, DeltaCycles))
continue;
SU.BotReadyCycle = CurrCycle - DeltaCycles;
return true;
}
Expand Down
3 changes: 3 additions & 0 deletions llvm/lib/Target/AIE/AIEMachineScheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ class AIEPostRASchedStrategy : public PostGenericScheduler {
SUnit *pickNodeAndCycle(bool &IsTopNode,
std::optional<unsigned> &BotEmissionCycle) override;

bool canOptimizeMemoryAccess(SUnit &SU, SchedBoundary &Zone,
const int DeltaCycle);

bool isAvailableNode(SUnit &SU, SchedBoundary &Zone,
bool VerifyReadyCycle) override;

Expand Down
Loading