Skip to content

Fixes bug in verilog name generation logic - #1016

Open
rdaly525 wants to merge 5 commits into
masterfrom
verilog-name
Open

Fixes bug in verilog name generation logic#1016
rdaly525 wants to merge 5 commits into
masterfrom
verilog-name

Conversation

@rdaly525

Copy link
Copy Markdown
Owner

Fixes #941

@rdaly525
rdaly525 requested review from joyliu37 and leonardt July 22, 2021 03:09

@joyliu37 joyliu37 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.

Looks good to me. So I need to use serializeToFile to dump the generator information?

->getMetaData()["verilog"];
module_name = make_name(instance_module->getName(), verilog_json);
}
if (instance_module->isGenerated() && instance_module->getGenerator()->getMetaData().count("verilog") > 0) {

@joyliu37 joyliu37 Jul 22, 2021

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.

After testing with clockwork, I need to change this line of code into

if (instance_module->isGenerated() && instance_module->getMetaData().count("verilog") > 0)

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.

@rdaly525 I think this means you need to check two separate cases here:

  1. the instance_module has "verilog" metadata
  2. the instance_module->getGenerator() has "verilog" metadata

@leonardt leonardt 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.

Change looks fine, but I think there's one extra case needed for Joey's use case

->getMetaData()["verilog"];
module_name = make_name(instance_module->getName(), verilog_json);
}
if (instance_module->isGenerated() && instance_module->getGenerator()->getMetaData().count("verilog") > 0) {

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.

@rdaly525 I think this means you need to check two separate cases here:

  1. the instance_module has "verilog" metadata
  2. the instance_module->getGenerator() has "verilog" metadata

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.

Add a Custom Name for CoreIR Generator When Generate Verilog

4 participants