Do not make models with no pin-to-pin relations constant generators #2058
Conversation
…nstant generators. Signed-off-by: Maciej Kurc <mkurc@antmicro.com>
| @@ -471,7 +471,7 @@ int mark_undriven_primitive_outputs_as_constant(AtomNetlist& netlist, int verbos | |||
| } | |||
| } | |||
|
|
|||
| if (!has_connected_inputs) { | |||
| if (!has_connected_inputs && !upstream_ports.empty()) { | |||
There was a problem hiding this comment.
This needs a comment explaining what !upstream_ports.empty() indicates (why is this not a constant generator if none of its inputs are connected to anything?). An example would be useful.
There was a problem hiding this comment.
Wouldn't a black box still have some inputs of the atom connected (so upstream_ports wouldn't necessarily be empty)? I am likely missing the point, but that means a comment with an example is needed I think.
|
@kmurray : this is a very short PR that would be good for you to take a look at as it's about timing analysis / graph. |
|
@vaughnbetz @kmurray Indeed I see that the change I've made is breaking. Basically what I wanted to achieve is to make VPR not consider output of black-box models as constant sources. This is needed (at least for now) for FPGA interchange where we do not have (currently) pin-to-pin relations defined. With the current VPR we see that all cell outputs are considered constants which makes the placer ignore all nets hence it hangs in an infinite loop. I thought that this change might be beneficial for VPR in general as it makes blackbox models more usable. |
|
@mkurc-ant : not quite sure how to proceed with this one. If you'd like to discuss it, it would be good to put on the Thursday meeting agenda. Since it's breaking it seems we can't put it back in its current form. |
Description
Currently for models with no pin-to-pin relations defined VPR assumes that their outputs become constant generators even though such models have inputs and they are connected to active logic.
This PR makes VPR stop implying that.
Related Issue
Motivation and Context
Generic blackbox support
How Has This Been Tested?
Types of changes
Not sure though how breaking the change is. Probably a discussion is needed.
Checklist: