Conversation
…instruction Remove explicit egress instructions for DATA_MOV reg->outport movements. The VectorCGRA hardware provides a direct bypass path from register banks to the routing crossbar, so no FU slot is needed for these movements. CTRL_MOV operations retain their explicit egress instructions. Add validateRegReadConstraints() to enforce the single-read-port hardware constraint: if a routing bypass and a computation both read from the same register cluster on the same tile at the same time step, they must read the identical register (same intra-index within the cluster). Update DFG collectHopRewrites() to skip node creation for DATA_MOV operations that no longer have corresponding instructions. Update YAML/ASM FileCheck patterns in 16 test files to reflect the new code generation output (first 50 lines, each PE block starts a new // ASM: / // YAML: check). Made-with: Cursor
|
hmmm,
I thought we still need the
|
Add validateRegReadConstraints() to enforce the hardware constraint: if a reg->outport routing bypass and a computation both read from the same register cluster (same tile, same time step), they must read the identical register. No constraint if from different clusters or if reg->port doesn't happen together with any computation. Track bypass reads via BypassRead struct during DATA_MOV expansion. The MOV instruction itself is kept unchanged. Update YAML/ASM FileCheck patterns in 5 affected test files. Made-with: Cursor
Thank you for the clarification! I have reverted the excessive changes. |
|
Shouldn't we work on mapping algorithm? i.e., sth like: https://github.com/coredac/dataflow/blob/c5e1f508ff06ae88a62aa5f495e240a53b7aafc5/lib/NeuraDialect/Mapping/mapping_util.cpp#L594 |
guosran
force-pushed
the
feature/data-mov-register-bypass
branch
2 times, most recently
from
f536c64 to
62971d3
Compare
Sry my bad, I have rewrote the code in |
guosran
force-pushed
the
feature/data-mov-register-bypass
branch
from
62971d3 to
4df691e
Compare
| Register *mlir::neura::getAvailableRegister(const MappingState &state, | ||
| Tile *tile, int start_time, | ||
| int exclusive_end_time) { | ||
| // Collect registers occupied by computations (non-MOV operations) on this |
| return reg; | ||
| for (int t = start_time; t < exclusive_end_time; ++t) { | ||
| auto op = state.getOpAt({reg, t}); | ||
| if (!op.has_value()) continue; |
There was a problem hiding this comment.
I don't like one liner for if, can we make this be:
if (!op.has_value()) {
continue;
}
| if (!op.has_value()) continue; | ||
| Operation *mapped_op = op.value(); | ||
| if (isa<neura::DataMovOp>(mapped_op) || isa<neura::CtrlMovOp>(mapped_op)) | ||
| continue; |
| if (!op.has_value()) continue; | ||
| Operation *mapped_op = op.value(); | ||
| if (isa<neura::DataMovOp>(mapped_op) || isa<neura::CtrlMovOp>(mapped_op)) | ||
| continue; |
| for (Register *reg : tile->getRegisters()) { | ||
| if (!state.isAvailableAcrossTimeInRange(reg, start_time, | ||
| exclusive_end_time)) | ||
| continue; |
| bool conflict = false; | ||
| for (Register *comp_reg : computation_regs) { | ||
| if (comp_reg->getRegisterFile() != candidate_file) | ||
| continue; |
| } | ||
|
|
||
| for (Register *reg : tile->getRegisters()) { | ||
| if (!state.isAvailableAcrossTimeInRange(reg, start_time, |
There was a problem hiding this comment.
Instead of collects the registers for non-MOV ops first, can we just update the isAvailableAcrossTime()? https://github.com/coredac/dataflow/blob/c5e1f508ff06ae88a62aa5f495e240a53b7aafc5/lib/NeuraDialect/Mapping/MappingState.cpp#L118
checking whether it is a register, and whether the register's cluster has any other register is occupied for either mov or compute?
Yes~ We use |
guosran
force-pushed
the
feature/data-mov-register-bypass
branch
from
4df691e to
62971d3
Compare
|
Sorry there is a branch is too messy... I will create a new PR. |
3 participants
Summary
This PR addresses issue #283 by enforcing the register-bank (RegisterFile) read constraint directly in the mapper’s global availability check. When a routing bypass (MOV) and a computation both read from the same register cluster on the same tile at the same time slot, they are forced to use the identical intra-index register, while MOV-only traffic remains unconstrained.
Changes
MappingState::isAvailableAcrossTimeinMappingState.cppto detectMOVvscomputeaccesses within the same RegisterFile and reject conflicting mixed accesses across different sibling registers in the same time slot.