Skip to content

Fix/noc backpressure#300

Open
guosran wants to merge 1 commit into
tancheng:masterfrom
guosran:fix/noc-backpressure
Open

Fix/noc backpressure#300
guosran wants to merge 1 commit into
tancheng:masterfrom
guosran:fix/noc-backpressure

Conversation

@guosran

@guosran guosran commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

Addresses #292 with a minimized LinkOr input-validity fix. This PR only updates LinkOrRTL.py and its focused test.

Changes

  • Gate xbar and FU messages by their own val bits before OR-ing payload/predicate fields.
  • Prevent an invalid stale FU message from polluting an active xbar message.
  • Add a concrete example as an inline test comment.

No Crossbar, Tile, new port, or new interface changes are included.

@tancheng tancheng requested review from tancheng and yyan7223 June 21, 2026 06:35
Comment thread noc/CrossbarRTL.py Outdated
Comment on lines +160 to +165
elif s.drain_when_inactive:
# An inactive route acts as a sink for stale traffic. The tile keeps
# this disabled after the current control has completed so tokens for
# the next control step are not dropped.
for i in range(num_inports):
s.recv_data[i].rdy @= 1

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I don't get this... Can we have a concrete example about why we need this?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I removed the Crossbar change. The Crossbar prologue behavior is now isolated in #303 with its own focused test and concrete inline example.

#300 now only changes LinkOrRTL.py: it gates FU/xbar messages by their own val bits before OR-ing payload/predicate, so stale invalid data cannot pollute the active valid input.

@guosran guosran force-pushed the fix/noc-backpressure branch from d5198fb to 53b83d3 Compare June 23, 2026 20:41
@guosran guosran force-pushed the fix/noc-backpressure branch from 53b83d3 to c0ff8db Compare June 25, 2026 14:43
@guosran guosran changed the base branch from kernel-submit to master June 25, 2026 14:51
@guosran guosran marked this pull request as draft June 25, 2026 14:53
@guosran guosran force-pushed the fix/noc-backpressure branch from c0ff8db to 85a3a32 Compare June 25, 2026 19:54
@guosran guosran force-pushed the fix/noc-backpressure branch from 85a3a32 to 32a563f Compare June 26, 2026 00:14
@guosran guosran marked this pull request as ready for review June 26, 2026 00:33
@guosran

guosran commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator Author

Concrete example:
LinkOrRTL should not let an uncommitted FU output win over routing-crossbar traffic.

  fu.val=1, fu_xbar_rdy=0
  xbar.val=1

Before:
output could include FU data even though the FU crossbar had not committed it.

After:
FU side is active only when fu.val && fu_xbar_rdy.
Xbar traffic can still pass through.

@yyan7223

Copy link
Copy Markdown
Collaborator

Concrete example: LinkOrRTL should not let an uncommitted FU output win over routing-crossbar traffic.

  fu.val=1, fu_xbar_rdy=0
  xbar.val=1

Before: output could include FU data even though the FU crossbar had not committed it.

After: FU side is active only when fu.val && fu_xbar_rdy. Xbar traffic can still pass through.

Sorry it's still not concrete enough. You can paste the waveform here to explain how an 'uncomitted' FU output win the traffic before you correction.

Comment thread noc/LinkOrRTL.py
# Interface
s.recv_fu = RecvIfcRTL(DataType)
s.recv_xbar = RecvIfcRTL(DataType)
s.fu_xbar_rdy = InPort(b1)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

s.recv_fu already contains a rdy signal, why would we still need s.fu_xbar_rdy?

Comment thread noc/LinkOrRTL.py
# which is guaranteed by the compiler/software.
s.send.msg.predicate @= s.recv_fu.msg.predicate | s.recv_xbar.msg.predicate
s.send.msg.payload @= s.recv_xbar.msg.payload | s.recv_fu.msg.payload
fu_active = s.recv_fu.val & s.fu_xbar_rdy

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is that a handshake check?

Comment thread noc/LinkOrRTL.py
Comment on lines +48 to +52
# Only let FU traffic win once the FU crossbar has actually committed
# the multicast/send for this cycle; otherwise the link can expose
# transient FU output and create cross-tile bubbles.
s.send.val @= fu_active | xbar_active
s.recv_fu.rdy @= s.send.rdy & s.fu_xbar_rdy

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I have similar feeling with @yyan7223, this part is a bit confusing. It sounds the root cause might not be here. Or we need a concrete example to show case why we need to change this file. The file originally should only target simple or functionality...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants