sv2v icon indicating copy to clipboard operation
sv2v copied to clipboard

Name of modules that contain parameterized types

Open kauser-rl opened this issue 3 years ago • 3 comments

I have a situation where I override the type of some signals via a parameter. The conversion to Verilog seems to be okay although for the default value of the type. I have two instances, one of which is non-default. There isn't a converted version of it. Any suggestions on how to work around it? I have pasted a test case below. There are three files, package, module with type and the wrapper that instances it.

mod_pkg.sv

`ifndef MOD_PKG
 `define MOD_PKG

package mod_pkg;

   typedef enum logic[1:0] {
     TYPEA_0  = 2'b00,
     TYPEA_1  = 2'b01,
     TYPEA_2  = 2'b10,
     TYPEA_3  = 2'b11
   } typea_t;

   typedef enum logic[2:0] {
     TYPEB_0  = 3'b000,
     TYPEB_1  = 3'b001,
     TYPEB_2  = 3'b010,
     TYPEB_3  = 3'b011,
     TYPEB_4  = 3'b100,
     TYPEB_5  = 3'b101,
     TYPEB_6  = 3'b110,
     TYPEB_7  = 3'b111
   } typeb_t;
endpackage // mod_pkg

mod_with_type.sv

`include "mod_pkg.sv"

module mod_with_type
  // Package imports
  import mod_pkg::*;
   #(parameter type  PR_TYPE = typea_t)
   (
    input wire PR_TYPE    type_i,
    output     logic      sel_o
   );

   parameter TYPE_W = $bits(typea_t);

   logic [TYPE_W-1:0] mask;

   generate if(PR_TYPE==typea_t) begin : g_typea
      assign mask = 2'b10;
   end else begin : g_typeb
      assign mask = 3'b101;
   end endgenerate

   assign sel_o = |(type_i & mask);

top.sv

module top
  // Package imports
  import mod_pkg::*;
   ();

   typea_t typea;
   typeb_t typeb;
   logic sel_a;
   logic sel_b;

   mod_with_type #(.PR_TYPE (typea_t)) u_typea (.type_i (typea), .sel_o (sel_a));
   mod_with_type #(.PR_TYPE (typeb_t)) u_typeb (.type_i (typeb), .sel_o (sel_b));
endmodule // top

kauser-rl avatar Sep 06 '22 11:09 kauser-rl

The converted top.v is pasted below. You can see two instances, but none of those modules exist or are generated by sv2v.

module top;
	wire [1:0] typea;
	wire [2:0] typeb;
	wire sel_a;
	wire sel_b;
	mod_with_type_3D46F u_typea(
		.type_i(typea),
		.sel_o(sel_a)
	);
	mod_with_type_17C7C u_typeb(
		.type_i(typeb),
		.sel_o(sel_b)
	);
endmodule

kauser-rl avatar Sep 06 '22 11:09 kauser-rl

sv2v typically needs to be presented with all source files in a single invocation to work correctly. In this specific example, top.sv and mod_with_type.sv must be converted "simultaneously." I recognize this requirement is not obvious. I'd like to improve the documentation and perhaps issue a warning in such circumstances.

A couple options:

A) sv2v top.sv mod_with_type.sv will print the desired output to stdout. You can also use -w output.v to write to a file. B) sv2v -w adj top.sv mod_with_type.sv will produce two files, top.v and mod_with_type.v, with the desired output.

zachjs avatar Oct 01 '22 19:10 zachjs

I haven't tested it, but providing multiple dependent files might be an issue. See the typical way to provide a list of files for compilation is via a .vc file which contains search paths for all modules. For sv2v, we have to create a list of all the files to convert one at a time. This is inconvenient, but it is still manageable. If we have to pull out all dependent modules, either this would need to be manually maintained or we would need our own script to do it. How about using the search path to look for the dependent modules, just like you would for the include files?

kauser-rl avatar Oct 03 '22 08:10 kauser-rl

It sounds like you’re interested in a flag akin to -y in other tools, automatically loading an unrecognized some_module from a source file like some_module.sv found in the library search paths. Is that correct? If so, I will look into implementing this feature.

Regardless of the above, sv2v expects that it will convert an entire design in a single invocation. Currently, this means specifying every source file as an input, e.g., sv2v rtl/**/*.sv as seen in some flows. With -y, it would ideally suffice to specify the path of the top module alone, set any appropriate search paths, and sv2v would output the entire converted design. Does this usage pattern seem reasonable?

Unlike other tools, sv2v would not implicitly search the working directory (-y .) by default, as this could break existing flows.

zachjs avatar Dec 26 '22 23:12 zachjs

A -y option will do it! Happy to test it if you want. With regards to breaking existing flows, as they won't be explicitly using -y on the command line, it should still be backwards compatible?

kauser-rl avatar Jan 03 '23 13:01 kauser-rl

Hi @zachjs - any update on when (if there is agreement on it) this option would be added?

kauser-rl avatar Feb 13 '23 13:02 kauser-rl

It may be a few weeks before I can begin work on this feature in earnest. Once I have a better timeline, I'll let you know.

zachjs avatar Feb 21 '23 14:02 zachjs

Thanks! I will eagerly wait for your update.

kauser-rl avatar Feb 21 '23 14:02 kauser-rl

I pushed an implementation of this feature as commit 882900c2ed94ad64179efb78c238cdb5d3718836 to branch libdir. Please let me know what you think!

zachjs avatar May 30 '23 00:05 zachjs

I have merged this into master as commit 51c90baf5ecbe5b33fe5e1dac31d80b7c1917f17. I am still very eager for your feedback!

zachjs avatar Jun 14 '23 03:06 zachjs

Sorry about the delay. Trying to get a release done. I'll get to this in a couple of weeks and let you know how it goes. It is on my TODO list!

kauser-rl avatar Jun 14 '23 08:06 kauser-rl

Do you mind creating a new release?

kauser-rl avatar Jun 14 '23 08:06 kauser-rl

I am interested in getting your feedback on the behavior before I generate an official release. This would ensure I don't commit to behavior that is confusing or not useful.

What aspect of the release eases your workflow? I can imagine any of the following being necessary: tag, GitHub release, build artifacts, or Hackage release. For example, I could make a v.test.0.0.0.1 tag for you. If that doesn't work, I can cut a release within the next couple weeks.

zachjs avatar Jun 15 '23 03:06 zachjs

I need to get our IT to install it on our compute cluster as a versioned module. A test tag is fine. The idea is that we can always identify on what version of tools we have qualified our design for reproducibility in the future.

kauser-rl avatar Jun 15 '23 08:06 kauser-rl

I pushed a test tag v0.test.1: https://github.com/zachjs/sv2v/releases/tag/v0.test.1. Thank you trying it out!

zachjs avatar Jun 16 '23 04:06 zachjs

Got the tag installed. It shows:

> sv2v --version
sv2v 0.0.10

When I run the test case I attached with this ticket, I don't see the expected output. Did I get the command wrong?

> sv2v -I `pwd` -y `pwd` -w adjacent top.sv
mod_with_type.sv: unfinished conditional directives: 1

Note sure what unfinished conditional directives: 1 means.

kauser-rl avatar Jun 16 '23 12:06 kauser-rl

That error refers to an unmatched ifdef or similar when the end of a file is reached. I will look into making that message more helpful.

zachjs avatar Jun 17 '23 13:06 zachjs

I just pushed a change to make that error message more useful as commit a05659cd06b7da57fd5758f4eba011dc8517dc4d and tag v0.test.2. There may well be an issue with sv2v, so please let me know what you find!

zachjs avatar Jun 17 '23 17:06 zachjs

Fixed the unmatched ifdef error. I'm able to get the correct output (pasted below). However I have a small request. It appears that that unique modules are created inline in the same file. It would be nice to have an option to split them out into their own files with the name of the file matching the module name. Currently we have a flow that relies on module names matching file names.

mod_with_type.v

module mod_with_type_17C7C (
	type_i,
	sel_o
);
	input wire [2:0] type_i;
	output wire sel_o;
	parameter TYPE_W = 2;
	wire [TYPE_W - 1:0] mask;
	generate
		if (TYPE_W == 2) begin : g_typea
			assign mask = 2'b10;
		end
		else begin : g_typeb
			assign mask = 3'b101;
		end
	endgenerate
	assign sel_o = |(type_i & mask);
endmodule
module mod_with_type_3D46F (
	type_i,
	sel_o
);
	input wire [1:0] type_i;
	output wire sel_o;
	parameter TYPE_W = 2;
	wire [TYPE_W - 1:0] mask;
	generate
		if (TYPE_W == 2) begin : g_typea
			assign mask = 2'b10;
		end
		else begin : g_typeb
			assign mask = 3'b101;
		end
	endgenerate
	assign sel_o = |(type_i & mask);
endmodule

kauser-rl avatar Jun 19 '23 09:06 kauser-rl

What if sv2v supported something like --write some/out/dir/, where it would automatically split the output into one module per file written into that directory? The directory structure of the input files would be ignored. Would this be a useful alternative to --write adjacent?

zachjs avatar Jun 19 '23 15:06 zachjs

That would be lovely! Is it quick to add? We are aiming to release end of this week.

kauser-rl avatar Jun 20 '23 08:06 kauser-rl

I've pushed a version of the above feature as 04d65bb3881b911e7b71d8ac86b4e74d356f5cfe and test tag v0.test.3. There are few limitations:

  • The output directory must already exist to convey intent given how I've overloaded the --write flag. I could imagine loosening this restriction before the official release. I'm also open to suggestions for a new name for a separate flag that would be mutually exclusive with --write.
  • Existing files in that directory are not removed, but files like some_module.v will automatically be overwritten if some_module exists in the output.
  • In verbose mode, trace comments outside of any module are not written to any file.
  • Use with --pass-through is forbidden.

Please let me know what you think of this new feature!

I'm inclined to think that this behavior is what I should have done for --write adjacent from the beginning, but I think it's too late to remove that feature.

zachjs avatar Jun 21 '23 12:06 zachjs

I don't think we will have any issues with your limitations. Let me get the version installed and test it. Thanks for quickly turning around.

kauser-rl avatar Jun 21 '23 13:06 kauser-rl

It works as expected. Thanks! Can we get a release tag please?

kauser-rl avatar Jun 21 '23 21:06 kauser-rl

Great to hear that it's working for you! I have made a release: v0.0.11. I think this was overdue. Please let me know if you run into any problems!

I also removed the test tags (v0.test.1, etc.) from earlier. I can restore them if necessary.

For my own reference and in case there is any interest, I've documented below what I did to make this release. I may try to script some of this in the future.

  1. make && make test
  2. Update sv2v.cabal and CHANGELOG.md to reference the new version.
  3. git commit -a -m "release v0.0.11" && git push origin master
  4. git tag -a v0.0.11 HEAD -m "Release v0.0.11" && git push origin v0.0.11
  5. stack sdist and upload the Hackage candidate.
  6. Create the GitHub release with the reformatted notes from the changelog.
  7. Publish the Hackage release.
  8. Ensure the GitHub Actions workflow created the release artifacts.

zachjs avatar Jun 22 '23 04:06 zachjs

@kauser-rl Is there anything more to work on for this issue?

zachjs avatar Jul 02 '23 19:07 zachjs

Nothing more. All good.

kauser-rl avatar Jul 02 '23 22:07 kauser-rl