Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Improve FST support to show enums #1358

Closed
veripoolbot opened this issue Oct 3, 2018 · 15 comments
Closed

Improve FST support to show enums #1358

veripoolbot opened this issue Oct 3, 2018 · 15 comments

Comments

@veripoolbot
Copy link
Contributor

@veripoolbot veripoolbot commented Oct 3, 2018


Author Name: Sergi Granell (@xerpi)
Original Redmine Issue: 1358 from https://www.veripool.org


Currently, the FST we generate is very limited (basically wires and one kind of scope). As FST actually supports SystermVerilog and VCD types and all kinds of scopes (such as module, package, struct, interface, etc), it would be nice to improve the FST support to make it generate these types.
It looks like this would requiere a major refactoring of the "declFoo" call dumping routines since fstapi needs more info than the currently passed on the current dumping functions parameters.

Some generic pseudo C++ code that would visit all the hierarchy and emit FST would look like:

void node_visit(const node_variable_t *node)
{
	enum fstVarType var_type;
	enum fstVarDir var_dir;

	switch (node->getType()) {
	case VAR_TYPE_WIRE:
		var_type = FST_VT_VCD_WIRE;
		break;
	case VAR_TYPE_SV_LOGIC:
		var_type = FST_VT_SV_LOGIC;
		break;
	...
	}

	if (node->getDir() == VAR_DIR_INPUT)
		var_dir = FST_VD_INPUT;
	else if (node->getDir() == VAR_DIR_INPUT)
		var_dir = FST_VD_OUTPUT;
	else
		var_dir = FST_VD_IMPLICIT;

	fstWriterCreateVar(m_fst, var_type, var_dir, node->getLength(), node->getName(), ...)
}

void node_visit(const node_struct_t *node)
{
	fstWriterSetScope(FST_ST_VCD_STRUCT, node->getName());

	for (auto &child: node->children()
		node_visit(child);

	fstWriterSetUpscope();
}

void node_visit(const node_module_t *node)
{
	fstWriterSetScope(FST_ST_VCD_MODULE, node->getName());

	for (auto &child: node->children()
		node_visit(child);

	fstWriterSetUpscope();
}

void emit_vars()
{
	node_visit(getRoot());
}
</code>
@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Oct 3, 2018


Original Redmine Comment
Author Name: Sergi Granell (@xerpi)
Original Date: 2018-10-03T14:05:24Z


The tricky part is, of course, that this code has to be generated and that the full hierarchy must also be available from the generated C++ code.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Oct 3, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-10-03T15:51:12Z


Do you want to try to fix the guts of Verilator to put out the types etc, or should I when I get time? (Meantime you could add the plumbing to the decl calls with defaulted arguments.)

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Oct 3, 2018


Original Redmine Comment
Author Name: Sergi Granell (@xerpi)
Original Date: 2018-10-03T16:28:10Z


Wilson Snyder wrote:

Do you want to try to fix the guts of Verilator to put out the types etc, or should I when I get time? (Meantime you could add the plumbing to the decl calls with defaulted arguments.)

I think it's better if you do that when you have time since I don't know the codebase very well and I'm not sure I'd be able to do that cleanly.

I'd like to help somehow, but I'm not sure what you mean by "add the plumbing to the decl calls with defaulted arguments".

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Oct 3, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-10-03T16:42:46Z


Ok, for signals, I propose I'll make the decl calls pass the right information. Can you then hook them up to the FST internal library?

For enums can you describe what FST wants?

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Oct 3, 2018


Original Redmine Comment
Author Name: Sergi Granell (@xerpi)
Original Date: 2018-10-03T16:51:58Z


Wilson Snyder wrote:

Ok, for signals, I propose I'll make the decl calls pass the right information. Can you then hook them up to the FST internal library?

For enums can you describe what FST wants?

Enums are a bit more tricky as you can see here: https://sourceforge.net/p/gtkwave/mailman/message/36430944/

If I understand the code correctly (https://github.com/nickg/nvc/blob/master/src/rt/fst.c), what Nick Gasson did is to set the enum variable as an string, and then use FST_VT_GEN_STRING + fstWriterEmitVariableLengthValueChange to change the string value. I guess the trick would be to emit a const lookup table from enum value to its name as an string (ENUM_FOO -> "ENUM_FOO").

I think a good idea would be to add an option to Verilator such as "--fst-emit-enum-names" that toggles between emitting the raw value or the string value. This shouldn't be a lot of trouble to implement when we have the basic infrastructure for the improved FST done.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Oct 3, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-10-03T23:51:40Z


First step, input/output/inout indications should now work.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Oct 4, 2018


Original Redmine Comment
Author Name: Sergi Granell (@xerpi)
Original Date: 2018-10-04T14:42:04Z


Wilson Snyder wrote:

First step, input/output/inout indications should now work.

Thanks, I've tested it and it works nice!

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Oct 5, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-10-05T00:25:15Z


Type of variables is now included. Done for now.

As to enums, keeping this bug open and on hold until GTKwave upstream's FST format gets the appropriate features to indicate enum information. This is in progress.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Oct 5, 2018


Original Redmine Comment
Author Name: Sergi Granell (@xerpi)
Original Date: 2018-10-05T05:46:46Z


Wilson Snyder wrote:

Type of variables is now included. Done for now.

As to enums, keeping this bug open and on hold until GTKwave upstream's FST format gets the appropriate features to indicate enum information. This is in progress.

Nice! Now it would be really useful if we could add struct support. I think it should be relatively easy: Add a "declStructStart(name?);" then all the "declFoo" called after this function mean that they are inside the struct, and to close the struct add a "declStructEnd()" callback.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Oct 8, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-10-08T11:24:02Z


GTKWave 3.3.95 just added support for enums in FSTs. Fixed Verilator to dump them with --trace-fst in git towards 4.006.

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Oct 8, 2018


Original Redmine Comment
Author Name: Sergi Granell (@xerpi)
Original Date: 2018-10-08T15:19:37Z


Wilson Snyder wrote:

GTKWave 3.3.95 just added support for enums in FSTs. Fixed Verilator to dump them with --trace-fst in git towards 4.006.

Awesome, thanks!

Btw looks like the enum variable has to be directly declared right?
So I'm trying with something like this:

typedef enum logic [3:0] {
	ALU_OP_ADD,
	ALU_OP_SUB,
	...
} alu_op_t;
</code>

And then using it as input to modules (input alu_op_t op) and buses but it doesn't get the enum (I guess the node isn't of type AstEnumDType but AstTypedef or similar?).

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Oct 8, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-10-08T22:38:00Z


What you indicated should work as an input, if it doesn't please attach an example. (And make sure building GTKWave from sources as you need yesterday's GTKWave.)

@veripoolbot
Copy link
Contributor Author

@veripoolbot veripoolbot commented Oct 27, 2018


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-10-27T12:43:49Z


In 4.006.

@fsiegle
Copy link

@fsiegle fsiegle commented Jan 21, 2020

I also try to dump enums in a FST file but without any success so far. Directions and types are outputted correctly, however, enums are just shown as vectors. A possible reason might be that the enums are inside of structs. To output them, I use the --trace-structs switch.

@wsnyder
Copy link
Member

@wsnyder wsnyder commented Jan 21, 2020

@fsiegle This bug was already closed, so please file a bug on enums in structs with --trace-structs and we'll take a look, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.