Open hofstee opened 5 years ago
Is it possible to share this verilog file so we can debug the performance?
Do you know if the issue is primarily in the pyverilog parsing step (text -> AST) or is it magma's processing of the AST?
I believe it's pyverilog parsing. I'm not entirely sure how/when magma processes the AST but it wasn't showing up in my profiling. If the conversion of the AST to magma is calling pyverilog's parse then it could be. It's just the full 32x16 cgra from garnet. It's also 10MB not 1MB.
Yea that makes sense, I think we need to look into using another parser. One option is to read the verilog into yosys, use the coreir backend, then parse the module interface from the coreir. I'll investigate how long it takes yosys to parse the verilog. Not sure if there's another open source parser with Python integration that is faster.
If we're doing shallow parsing, we only need the modules specified in the target_modules
right? So can't you just do something like
for mod in target_modules:
magma.DefineFromVerilog(re.search(f"module {mod} \([^)]*\);(?s)*?endmodule"))
Hmm, clever, I think there's still the issue where the types of the ports are declared in the body of the module, e.g.
module foo(a, b);
input a;
output b;
...
endmoule
But we could have an option for that if you know the module declaration is in the above form (does it handle newlines in the parameter list?)
I think the two best options are (a) trying a new/better parser e.g. yosys, or (b) trying out the microgrammar approach (see https://web.stanford.edu/~mlfbrown/paper.pdf).
I really think the latter wouldn't be too hard and would be really useful.
However, to unblock yourself @THofstee you could definitely write a custom function to parse according to your regex rule if it functionally works.
For a 32x16 CGRA (~1MB Verilog file), it takes over 90 seconds to get through the parse step of magma. I really only want the top
Garnet
module's interface to be parsed. My workaround is to just regex search formodule Garnet \([^)]*\);
and tack on\nendmodule
to it andDefineFromVerilog
instead. This takes about 0.02 seconds.Could we add the ability to parse headers only of just the modules listed in
target_modules
?