Skip to content
New issue

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

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

Already on GitHub? # to your account

Feature: Import external module #9

Merged
merged 12 commits into from
Jan 6, 2024
Merged

Feature: Import external module #9

merged 12 commits into from
Jan 6, 2024

Conversation

khwong-c
Copy link
Collaborator

@khwong-c khwong-c commented Jan 3, 2024

Closing #7

Adding Capability to import external SV.
I/O and Parameter parsing is done automatically.

@yaus
Copy link
Contributor

yaus commented Jan 4, 2024

    def test_complex_param_propagation(self):
        code = """
        `define ABC 12+3
        module Mod2 #(
            parameter DEPTH = `ABC,
            parameter WIDTH = $clog2(DEPTH)
        ) (
            input clk,
            input wen,
            output signed [WIDTH-1:0] dout
        );
        endmodule
        """
        mod = ExternalModule.from_code(code, "Mod2")(DEPTH=16)
        assert mod.params == {"WIDTH": 4, "DEPTH": 16}
        assert len(mod.io.dout) == 4

This is a complex test that cannot pass.
By this test case, I wonder if need to keep those complex detection functions in the code base.

@khwong-c
Copy link
Collaborator Author

khwong-c commented Jan 4, 2024

This one shouldn't pass due to the following

  • We use pyverilog to parse the SV/Verilog file
  • pyverilog make use of the iverilog binary as the Preprocessor, which handle the directives.
  • I bypassed the preprocessing part as I don't want to bring in Binary Dependency at the moment.

I got stuck a bit in this one as I try to keep the library simple and easy to install.
However, I couldn't tell how often a directive appears in a top level.

Do you have any advice on this one?

@khwong-c
Copy link
Collaborator Author

khwong-c commented Jan 4, 2024

My current thought on this is: we might need to bring in some binary dependency if we want to have better support on module import.

  • pyverilog is too old and requires iverilog binary for better module import support
  • I couldn't find a pure-python SV parser implementation ATM.
  • We should switch to a better SV parser if binary dependency is not avoidable.

I am currently looking into hdlConvertor
We need to install antlr4 in this case.
apt install antlr4

@yaus
Copy link
Contributor

yaus commented Jan 6, 2024

I think antlr4 is a better choice compared with PyVerilog.

This allows the whole package still functional when we are running on other platform
(e.g. Windows, Mac, etc.) with single command `pip install magia` installation
@khwong-c
Copy link
Collaborator Author

khwong-c commented Jan 6, 2024

We have moved to ANTLR4 (hdlConvertor) and make this feature optional. The above test case shall pass.

The reason to make this feature optional is we want most of the features from the library to be platform-independent.
However, the current implementation can only run on X86_64 Linux.
Therefore, we turn this feature into optional.

This feature can considered as a corner use case, as discussed with @yaus, we will put ExternalModule to a lower priority.

@khwong-c khwong-c merged commit a32a897 into main Jan 6, 2024
@khwong-c khwong-c deleted the feature/external-module branch January 24, 2024 21:37
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants