Skip to content

Allow padded XSI input feed for FIFO sim - #1613

Open
ollycassidy13 wants to merge 1 commit into
Xilinx:devfrom
ollycassidy13:split/xsi-padded-input-feed
Open

Allow padded XSI input feed for FIFO sim#1613
ollycassidy13 wants to merge 1 commit into
Xilinx:devfrom
ollycassidy13:split/xsi-padded-input-feed

Conversation

@ollycassidy13

Copy link
Copy Markdown

Allows padded XSI input feeds during FIFO simulation.
This is a low-level XSI runtime compatibility fix for stream padding behavior.
Change type: C++

@STFleming STFleming left a comment

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.

Thanks @ollycassidy13.

It looks like this is increasing the number of inferences that are pushed into rtlsim_xsi.cpp which is used during FIFO depth simulation.

I would argue that this is already a feature built into FINN with the fifosim_n_inferences builder argument.

fifosim_n_inferences: Optional[int] = 2

However maybe I'm overlooking a use case that requires this?

It also looks like this is increasing the number of input inferences without increasing the checking logic for how many outputs are observed and they get flagged as spurious outputs and things will error out.

@ollycassidy13

Copy link
Copy Markdown
Author

The requirement is to feed N + padding inputs but capture exactly N outputs; setting fifosim_n_inferences to N + padding would also wait for the padded outputs and is not equivalent.

For a single output stream, the current code exits as soon as the Nth output completes, so it will not observe a padded output as spurious. This could be changed to handle multiple output streams if required (deassert each output stream’s tready as soon as it reaches N)?

@STFleming

Copy link
Copy Markdown
Collaborator

Thanks that clears things up a bit @ollycassidy13!

I think I'm finding the term padding a bit misleading here, to me this would mean resizing within a tensor/frame of a single inference to add some padding, but this is essentially increasing the number of input inferences (i.e. it increases the number of complete input frames) if I understand it correctly?

I'm also still a little unsure why you need to feed in N + padding inputs and collect only N outputs instead? Can you explain the motivation for the PR a bit more please.

@ollycassidy13

Copy link
Copy Markdown
Author

The FIFO characterization needs continued input traffic while the first inference traverses the pipeline so it can observe overlapping activity and peak FIFO occupancy. Using only one input frame removes input traffic too early. Setting fifosim_n_inferences=2 is not equivalent because it also requires a second output, and the TinyDeiT loop body stalled in the tail of that second dummy inference.

The intended behavior is:

  1. feed 2 complete input frames
  2. measure FIFO activity during the overlap
  3. finish after receiving 1 complete output frame

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.

2 participants