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

Implement logic for fork/join #98

Closed
hs-apotell opened this issue Nov 15, 2019 · 8 comments
Closed

Implement logic for fork/join #98

hs-apotell opened this issue Nov 15, 2019 · 8 comments

Comments

@hs-apotell
Copy link
Contributor

Attached is an example of file that fails to parse.
top.zip

@Thomasb81
Copy link
Contributor

It seems to be that we miss test of syntax like:

integer x, y, z;
  fork //set a seed at the start of a thread
  begin process::self.srandom(100); x = $urandom; end
  //set a seed during a thread
  begin y = $urandom; process::self.srandom(200); end
  // draw 2 values from the thread RNG
  begin z = $urandom + $urandom ; end
join

Ieee2017.1 page 541, which are similar to the construction of the testcase.

@hs-apotell I do not have the possibility to test now, but I think that it would work if you name your fork, which sound to me an acceptable temporary work-arround (if it works) :

  fork : my_fork
    forever begin
      #10 a = m | n | o;
      accelerate = a | b | c;
      #10 b = p | q | r;
      accelerate = a | b | c;
      #10 c = t | u | v;
      accelerate = a | b | c;
    end
    forever begin
     #4 p = ~p;
     #4 q = ~q;
     #3 r = ~r;
     #2 s = ~s;
     #5 t = ~t;
     #3 u = ~u;
     #5 v = ~v;
    end
  join

@Thomasb81
Copy link
Contributor

OK so definitively, @Nic30 was right in this analyze. The grammar is ok. The error message come from the notImplemented call.

Unless you want explore what is in this block and you have to implement something instead of calling the NottImplemented method or you can turn off the debug mode. The "problem" is then silently ignored.

What are your expectation regarding those 2 blocks ?

@hs-apotell
Copy link
Contributor Author

@Thomasb81 I tried your suggestion about naming the fork but I still get the same log message error.

top.v:49:2: VerStatementParser.visitStatement_item.par_block Conversion to Python object not implemented
    ...fork:whateverforeverbegin#10a=m|n|o;accelerate=a|b|c;#10b=p|q|r;accelerate=a|b|c;#10c=t|u|v;accelerate=a|b|c;endforeverbegin#4p=~p;#4q=~q;#3r=~r;#2s=~s;#5t=~t;#3u=~u;#5v=~v;endjoin...

I am not sure how this affects the output. I haven't dug any deeper on this.

@hs-apotell
Copy link
Contributor Author

What are your expectation regarding those 2 blocks ?

@Thomasb81 I need to know all occurrences/use of very signal in the code i.e. post elaboration. After that error message, I am not sure whether or not the definition within that block is parsed or not.

@Nic30
Copy link
Owner

Nic30 commented Nov 16, 2019

I am not sure how this affects the output. I haven't dug any deeper on this.

There is nop instead of that parallel block. Parsing there https://github.com/Nic30/hdlConvertor/blob/master/src/svConvertor/statementParser.cpp#L199 should be done very similarly as there https://github.com/Nic30/hdlConvertor/blob/master/src/svConvertor/statementParser.cpp#L209
https://github.com/Nic30/hdlConvertor/blob/master/src/svConvertor/statementParser.cpp#L590
We just need some flag to note block type.

I'll do it after fixing of #90 if it will be still an issue.

@Thomasb81
Copy link
Contributor

May be we should agree on the definition of what we understand by "parse": hdlConvertor is base on Antlr4 library. This library ease the parser development. Like other library of this kind it relies on token and grammar description. For hdlConvertor those files are : sv2017Parser.g4 sv2017Lexer.g4 sv2017verilogPreprocLexer.g4 verilogPreprocParser.g4

They are used to build software FSM in the target language.

When we say your code is correctly parsed It means the code successfully pass all the trigged state of the FSM ; it match the SystemVerilog syntax : we can eventually access the data in a meaning full way (AST ). But at this step the meaning is not guaranty.

To give additional value to the tool, you should give meaning and use the data resulting of this AST. It is still an huge and very specific job, not all data are use for every task related to hdl language processing. In your case when the processing reach NotImplementedLogger::print. It is just a message saying : " In the default application, we do not do any thing with data encounter in a fork / join construction "

So Say differently you can put everything you want until it match SV syntax it is fine in the fork / join block, it will actually be fine.

As far as I know hdlConvertor is not doing any "meaning check", there is no elaboration.
I guess that what @Nic30 is proposing you, is to enhance the tool with a default behavior to capture what inside the fork / join block in the context data structure return by the tool. It will allow you to use it in your application.

Hoping this explanation tentative is useful.

By the way, if you rename your file top.sv and drop it in tests/sv_test/std2017 and re-run the relevant test:
$ python -m unittest tests/test_sv2017_std_examples_parse.py

You should observe that one test has been added and all test are pass. That's a way to check the tool understand correctly the syntax of your file.

@hs-apotell
Copy link
Contributor Author

I mostly agree with your assessment that what's useful is very application specific. However, HDL being a "middleware", I would still expect anything that's in the AST to be converted into HDL classes. I shouldn't have to reach into the AST to get that information. Isn't that the whole purpose of implementing HDL - generic wrapper that presents VHDL, Verilog and System Verilog constructs in similar fashion.

In the default application, we do not do any thing with data encounter in a fork / join construction

HDL is just a library so nothing really has a use for anything. It is merely an generic interface to language constructs. The implementation is treating the Python extension as the application. It cannot make decision about what is and what is not useful to applications using the library. Everything in the AST should be converted to HDL constructs.

So, the question is - Is the body of the fork/join converted and accessible via HDL constructs or not? And, as I dug deeper, it looks like the constructs are available. The messaging, however, is little misleading because it is specific to the notion of Python extension being the application.

@Thomasb81
Copy link
Contributor

So, the question is - Is the body of the fork/join converted and accessible via HDL constructs or not?

Answer is no, see
#96 (comment)

you have to replace following statement :

	if (pb) {
		NotImplementedLogger::print(
				"VerStatementParser.visitStatement_item.par_block", pb);
		return create_object<HdlStmNop>(ctx);
	}

by

	if (pb) {
		return visitParBlock(pb);
	}

Where you have to provide the visitParBlock method

Nic30 added a commit that referenced this issue Mar 31, 2020
@Nic30 Nic30 closed this as completed Mar 31, 2020
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

No branches or pull requests

3 participants