Skip to content

[LibArchFPGA] Logical Models More Cleanups #3012

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
5 tasks
AlexandreSinger opened this issue Apr 30, 2025 · 1 comment
Open
5 tasks

[LibArchFPGA] Logical Models More Cleanups #3012

AlexandreSinger opened this issue Apr 30, 2025 · 1 comment

Comments

@AlexandreSinger
Copy link
Contributor

Recently, the way that the logical models were stored in VTR has changed to remove pointer accesses to the logical models and give each model a unique ID: #3004

The goal of the original PR was to remove how models were being stored as linked lists. There are still some further cleanups which would make the Logical Models interface even better.

Logical Model Data Structure

struct t_model_ports {
enum PORTS dir = ERR_PORT; /* port direction */
char* name = nullptr; /* name of this port */
int size = 0; /* maximum number of pins */
int min_size = 0; /* minimum number of pins */
bool is_clock = false; /* clock? */
bool is_non_clock_global = false; /* not a clock but is a special, global, control signal (eg global asynchronous reset, etc) */
std::string clock; /* The clock associated with this pin (if the pin is sequential) */
std::vector<std::string> combinational_sink_ports; /* The other ports on this model which are combinationally driven by this port */
t_model_ports* next = nullptr; /* next port */
int index = -1; /* indexing for array look-up */
};
/**
* @brief Struct containing the information stored for a logical model in the
* LogicalModels storage class below.
*/
struct t_model {
char* name = nullptr; ///< name of this logic model
t_model_ports* inputs = nullptr; ///< linked list of input/clock ports
t_model_ports* outputs = nullptr; ///< linked list of output ports
void* instances = nullptr; ///< TODO: Remove this. This is only used in the Parmys plugin and should be moved into there.
int used = 0; ///< TODO: Remove this. This is only used in the Parmys plugin and should be moved into there.
vtr::t_linked_vptr* pb_types = nullptr; ///< Physical block types that implement this model
bool never_prune = false; ///< Don't remove from the netlist even if a block of this type has no output ports used and, therefore, unconnected to the rest of the netlist
};

  • The t_model and t_model_ports store their names as C-style strings (char*), these should be upgraded to std::strings.
  • The input and output ports of the t_model are stored as linked lists, however after the model is constructed, these are never changed. These should be vectors (hence, index in the t_model_ports can also be removed).
  • instances and used members of t_model are only used in the Parmys plugin and should be moved out of this class.
  • pb_types is stored as a linked list as well. Similar to the input and output ports this should be a simple vector.

Parmys Plugin Order Dependence

  • The prior cleanup of the models ended up reversing the order that the models were stored in VTR. This did not cause any issues within VTR; however Parmys' results changed slightly. The results were correct (as far as I could tell), but they were just different. To get around this and make testing easier, I just reversed the list of models when reading it into Parmys. This reversing should not be done and the golden results should be regenerated.

// After a change to the construction of the user models, the order was
// reversed, which slightly changed the results. Reversing them back to
// attain the same results, simplifying my testing.
// TODO: Regenerate the golden solutions to use the new model order.
std::vector<LogicalModelId> user_models(Arch.models.user_models().begin(), Arch.models.user_models().end());
std::reverse(user_models.begin(), user_models.end());
for (LogicalModelId model_id : user_models) {

// After a change to the construction of the user models, the order was
// reversed, which slightly changed the results. Reversing them back to
// attain the same results, simplifying my testing.
// TODO: Regenerate the golden solutions to use the new model order.
std::vector<LogicalModelId> user_models(Arch.models.user_models().begin(), Arch.models.user_models().end());
std::reverse(user_models.begin(), user_models.end());
for (LogicalModelId model_id : user_models) {

// After a change to the construction of the user models, the order was
// reversed, which slightly changed the results. Reversing them back to
// attain the same results.
// TODO: Regenerate the golden solutions to use the new model order.
std::vector<LogicalModelId> user_models(arch.models.user_models().begin(), arch.models.user_models().end());
std::reverse(user_models.begin(), user_models.end());
for (LogicalModelId model_id : user_models) {

@AlexandreSinger
Copy link
Contributor Author

Another thing to add:

  • There are many parts of the code that use the LogicalModelIds of each of the models in the model library. To use these it uses the get method based on the name; however these IDs are always the same. These can be made into static constexpr members which could greatly improve performance.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant