Skip to content

[Buffers] fix non-CFDFC paths having non-zero occupancy #979

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.

Sign up for GitHub

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

Open
ziadomalik wants to merge 4 commits into
base: main
Choose a base branch
from
Open

[Buffers] fix non-CFDFC paths having non-zero occupancy #979

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6681142
chore: fix non-CFDFC paths having non-zero occupancy
ziadomalik Jun 13, 2026
02cb855
chore: clang-format
ziadomalik Jun 15, 2026
8cc3807
chore: add per-cfdfc ii into the constraint
ziadomalik Jul 13, 2026
e142eab
chore: clang-format
ziadomalik Jul 13, 2026
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
9 changes: 9 additions & 0 deletions include/dynamatic/Transforms/BufferPlacement/Utils/BufferPlacementMILP.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,15 @@ class BufferPlacementMILP : public MILP<BufferPlacement> {
void addBackedgeConstraints(ArrayRef<CFDFC *> cfdfcs,
llvm::MapVector<Value, CPVar> &channelOccupancy);

/// [FPGA24] Adds path-level occupancy equality constraints for reconvergent
/// paths that are entirely within a CFDFC (Paper: Section 5, Equations
/// 10-11). We skip paths with forks outside all CFDFC's.
void addPathOccupancyEqualityConstraints(
ArrayRef<fpga24::ReconvergentPathWithGraph> reconvergentPaths,
ArrayRef<CFDFC *> cfdfcs,
const llvm::MapVector<CFDFC *, double> &cfdfcIIs,
llvm::MapVector<Value, CPVar> &channelOccupancy);

/// [FPGA24] Adds imbalance constraints for reconvergent paths in LP1.
void addReconvergentPathConstraints(
ArrayRef<fpga24::ReconvergentPathWithGraph> reconvergentPaths);
Expand Down
48 changes: 28 additions & 20 deletions lib/Transforms/BufferPlacement/FPGA24Buffers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "mlir/IR/Value.h"
#include "mlir/Support/LLVM.h"
#include "mlir/Support/LogicalResult.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Debug.h"
Expand Down Expand Up @@ -150,51 +151,58 @@ void OccupancyBalancingLP::setup() {
return;
}

/// Target II = 1 for maximum throughput
double targetII = latencyResult.targetII;
if (targetII <= 0.0) {
targetII = 1.0;
}
/// Create variables for each channel
/// N_c: Maximal token occupancy on channel c.

/// (Paper: Section 5, Table 2)
/// N_c: Maximal token occupancy on channel c.
this->addOccupancyVars(allChannels, channelOccupancy, MAX_OCCUPANCY);

/// (Paper: Section 5, Equation 8): N_c >= L_c / II
/// We enforce this for the global II, but also for each CFDFC's specific II
/// to ensure sufficient buffering in faster loops.
/// (Paper: Section 5, Equation 15): Making the required occupancy the
/// maximum of all CFDFCs' II.
/// Determine which channels belong to at least one CFDFC.

@Jiahui17 Jiahui17 Jul 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you add a remark saying why we care about this? (i guess this is for setting the occupancy to be 1 for every channel that is not part of a cfdfc, but has latency > 0?

llvm::DenseSet<Value> cfdfcChannels;
for (CFDFC *cfdfc : cfdfcs)
for (Value channel : cfdfc->channels)
cfdfcChannels.insert(channel);

/// (Paper: Section 5, Equation 8): N_c >= L_c / II
/// Per-channel lower bound ensuring the pipeline has enough tokens in flight.
/// For non-CFDFC channels (executed once), N_c >= 1 suffices.
llvm::MapVector<Value, double> requiredOccupancy;

// Initialize with global II constraint
for (Value channel : allChannels) {
unsigned latency = latencyResult.channelExtraLatency.lookup(channel);
requiredOccupancy[channel] = static_cast<double>(latency) / targetII;
if (cfdfcChannels.contains(channel))
requiredOccupancy[channel] = static_cast<double>(latency) / targetII;
Comment on lines +175 to +176

@Jiahui17 Jiahui17 Jul 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this a duplicate of the logic below?

else
requiredOccupancy[channel] = (latency > 0) ? 1.0 : 0.0;
}

// Update by taking the maximum of the per-CFDFC constraints
/// (Paper: Section 5, Equation 15): Take the maximum across per-CFDFC IIs.
for (CFDFC *cfdfc : cfdfcs) {
double cfdfcII = latencyResult.cfdfcTargetIIs.lookup(cfdfc);
if (cfdfcII < 1.0)
continue;

for (Value channel : cfdfc->channels) {
if (requiredOccupancy.count(channel)) {
unsigned latency = latencyResult.channelExtraLatency.lookup(channel);
double specificOcc = static_cast<double>(latency) / cfdfcII;
if (specificOcc > requiredOccupancy[channel]) {
requiredOccupancy[channel] = specificOcc;
}
}
if (!requiredOccupancy.count(channel))
continue;
unsigned latency = latencyResult.channelExtraLatency.lookup(channel);
double specificOcc = static_cast<double>(latency) / cfdfcII;
if (specificOcc > requiredOccupancy[channel])
requiredOccupancy[channel] = specificOcc;
}
}

addMinOccupancyConstraints(requiredOccupancy, channelOccupancy);
addBackedgeConstraints(cfdfcs, channelOccupancy);
addChannelPropertyOccupancyConstraints(channelOccupancy);

/// (Paper: Section 5, Equations 10-11): Path occupancy equality.
addPathOccupancyEqualityConstraints(reconvergentPaths, cfdfcs,
latencyResult.cfdfcTargetIIs,
channelOccupancy);

/// (Paper: Section 5, Equation 14): Minimize sum(B_c * N_c).
this->setOccupancyBalancingObjective(channelOccupancy);

markReadyToOptimize();
Expand Down
72 changes: 72 additions & 0 deletions lib/Transforms/BufferPlacement/Utils/BufferPlacementMILP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1350,6 +1350,78 @@ void BufferPlacementMILP::addBackedgeConstraints(
}
}

void BufferPlacementMILP::addPathOccupancyEqualityConstraints(
ArrayRef<fpga24::ReconvergentPathWithGraph> reconvergentPaths,
ArrayRef<CFDFC *> cfdfcs, const llvm::MapVector<CFDFC *, double> &cfdfcIIs,
llvm::MapVector<Value, CPVar> &channelOccupancy) {

for (auto [pathIdx, pathWithGraph] : llvm::enumerate(reconvergentPaths)) {
Comment thread
ziadomalik marked this conversation as resolved.
const ReconvergentPath &rp = pathWithGraph.path;
const CFGTransitionSequenceSubgraph *graph = pathWithGraph.graph;
const DataflowGraphNode &forkNode = graph->nodes[rp.forkNodeId];

// We skip paths whose fork is outside all CFDFCs.
if (forkNode.type == DataflowGraphNode::REGULAR &&
llvm::none_of(cfdfcs, [&](CFDFC *cfdfc) {
return cfdfc->units.contains(forkNode.op);
}))
continue;

std::vector<SimplePath> simplePaths =
enumerateSimplePaths(*graph, rp.forkNodeId, rp.joinNodeId, rp.nodeIds);
if (simplePaths.size() < 2)
continue;

// For each simple path, the sum of channel occupancies (N_c) and the total
// latency of the units.
std::vector<std::pair<LinExpr, double>> pathTerms;
for (const auto &path : simplePaths) {
LinExpr occupancySum;
double latencySum = 0.0;

for (NodeIdType nodeId : path.nodes) {
if (nodeId == rp.forkNodeId || nodeId == rp.joinNodeId)
continue;
const DataflowGraphNode &node = graph->nodes[nodeId];
if (node.type != DataflowGraphNode::REGULAR)
continue;
auto latOrFail =
timingDB.getLatency(node.op, SignalType::DATA, targetPeriod);
if (succeeded(latOrFail) && *latOrFail > 0.0)
latencySum += *latOrFail;
}

for (EdgeIdType edgeId : path.edges) {
Value channel = graph->edges[edgeId].channel;
if (channelOccupancy.count(channel))
occupancySum += channelOccupancy[channel];
}

pathTerms.emplace_back(std::move(occupancySum), latencySum);
}
Comment on lines +1359 to +1401

@Jiahui17 Jiahui17 Jul 29, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I couldn't convince myself that this logic corresponds to

Image

Does it make sense to do something like:

for (auto cf : CFDFCs)

  for (auto rp : reconvergentPaths) {
    if (/* the BB sequence of "rp" is contained in the CFDFC */)
      /* add occupancy balancing constraint */
  }


// (Paper: Section 5, Equation 11): Occupancy(p0) = Occupancy(pi)
for (auto [cfdfcIdx, cfdfc] : llvm::enumerate(cfdfcs)) {
if (forkNode.type == DataflowGraphNode::REGULAR &&
!cfdfc->units.contains(forkNode.op))
continue;

double cfdfcII = std::max(1.0, cfdfcIIs.lookup(cfdfc));

for (size_t i = 1; i < pathTerms.size(); ++i) {
auto &[vars0, lat0] = pathTerms[0];
auto &[varsI, latI] = pathTerms[i];

std::string name =
llvm::formatv("pathOccEq_{0}_cfdfc{1}_{2}", pathIdx, cfdfcIdx, i)
.str();
model->addConstr(vars0 + lat0 / cfdfcII == varsI + latI / cfdfcII,
name);
}
}
}
}

void BufferPlacementMILP::addReconvergentPathConstraints(
ArrayRef<fpga24::ReconvergentPathWithGraph> reconvergentPaths) {
for (size_t pathIdx = 0; pathIdx < reconvergentPaths.size(); ++pathIdx) {
Expand Down
Loading