pulp-platform / morty Goto Github PK

If the source files contain multiple declarations of modules with the same name, the module graph will be built in a strange way. This should only impact the graph_file output and the output if filtered for a specific top module.

Is it possible to add support for in-order/global defines?

#mydefines.sv:

// This would take presedence until undef, or new value defined
`define DW=32

#mymodule.sv

// Define from previous compiles are available, unless undef
module mymodule (
 input [`DW-1:0] data
);
endmodule

morty mydefine.sv mymodule.sv

sv-parser gobbles comments after String literals into its whitespace vector. Consequently, they are not stripped from the sources. A minimal example:

// General comment
module comment_module #(
    parameter blub = "blub"    // "blub" or "blub1"
) (
    // In body comment.
);

endmodule

Produces:

module blub_comment_module #(
    parameter blub = "blub"    // "blub" or "blub1"
) (
    
);

endmodule

Expect:

module blub_comment_module #(
    parameter blub = "blub" 
) (
    
);

endmodule

Preprocessed syntax tree:

PreprocessorText { nodes: ([Comment(Comment { nodes: (Locate { offset: 0, line: 1, len: 18 },) }), NotDirective(SourceDescriptionNotDirective { nodes: (Locate { offset: 18, line: 1, len: 47 },) }), StringLiteral(StringLiteral { nodes: (Locate { offset: 65, line: 3, len: 6 }, [Space(Locate { offset: 71, line: 3, len: 4 }), Comment(Comment { nodes: (Locate { offset: 75, line: 3, len: 20 },) }), Space(Locate { offset: 95, line: 3, len: 1 })]) }), NotDirective(SourceDescriptionNotDirective { nodes: (Locate { offset: 96, line: 4, len: 8 },) }), Comment(Comment { nodes: (Locate { offset: 104, line: 5, len: 19 },) }), NotDirective(SourceDescriptionNotDirective { nodes: (Locate { offset: 123, line: 5, len: 43 },) })],) }

Hi there!

Morty throws a parse error on this file:

module mymodule (
    y,
    a,
    b
);
  parameter WIDTH = 1;
  output [0:WIDTH-1] y;
  input [0:WIDTH-1] a;
  input [0:WIDTH-1] b;

  genvar i;

  generate
    begin : t
      for (i = 0; i < WIDTH; i = i + 1) begin : w

        assign y[i] = a[i] ^ b[i];

      end  // block: w
    end

  endgenerate
endmodule

The error message is

Error: parse error
   --> main.v:14:10
   |
14 |     begin : t
   |          ^
Error: Parse error: Some(("main.v", 168))

Unfortunately the a2o repository is full of such occurrences.

Thanks!

Hi there!

When Morty pickles a header file such as

`ifndef BP_COMMON_CACHE_ENGINE_SVH
`define BP_COMMON_CACHE_ENGINE_SVH

typedef logic newtype_t;

`endif

then it becomes

`define BP_COMMON_CACHE_ENGINE_SVH

typedef logic newtype_t;

and this may cause tools to crash if the header is included multiple times.
I created an example here.

What workarounds would you suggest? Or is there anything that can be done in Morty to fix that problem?

I'm quite surprised to not have stumbled into this problem before so maybe I'm doing something wrong here.
Maybe doing such a typedef in a svh is also not legal, but can be found in the wild such as here.

Thanks!
Flavien

Parsing of sv source files fails for blocks that contain a case statement, where the keys are defines. To highlight a few examples that fail from the pulp-platform:
https://github.com/pulp-platform/apb_adv_timer/blob/master/rtl/adv_timer_apb_if.sv
https://github.com/pulp-platform/apb_gpio/blob/master/rtl/apb_gpio.sv
https://github.com/pulp-platform/udma_core/blob/master/rtl/common/udma_ctrl.sv

Is this behaviour intentional?

This largely depends on the sv-parser preprocessor's ability to keep comments (dalance/sv-parser#5).

There seems to be a related issue that even the parser is dropping comments in the syntax tree.

Hi!

Thank you for providing Morty, this is a very useful tool!
Some generate statements seem to be not accepted by Morty, whereas they seem to be accepted by Verilator.

This is an example from OpenTitan:

// Copyright lowRISC contributors.
// Licensed under the Apache License, Version 2.0, see LICENSE for details.
// SPDX-License-Identifier: Apache-2.0

// This file is auto-generated.


// This is to prevent AscentLint warnings in the generated
// abstract prim wrapper. These warnings occur due to the .*
// use. TODO: we may want to move these inline waivers
// into a separate, generated waiver file for consistency.
//ri lint_check_off OUTPUT_NOT_DRIVEN INPUT_NOT_READ
module prim_flop_2sync #(

  parameter int               Width      = 16,
  parameter logic [Width-1:0] ResetValue = '0

) (
  input                    clk_i,
  input                    rst_ni,
  input        [Width-1:0] d_i,
  output logic [Width-1:0] q_o
);

  if (1) begin : gen_generic
    prim_generic_flop_2sync #(
      .Width(Width),
      .ResetValue(ResetValue)
    ) u_impl_generic (
      .*
    );

  end

endmodule
//ri lint_check_on OUTPUT_NOT_DRIVEN INPUT_NOT_READ

If we remove the if(1), then Morty complains:

Error: parse error
   --> src/prim_flop_2sync.sv:13:16
   |
13 |    generate begin : gen_generic
   |                ^
Error: Parse error: Some(("prim_flop_2sync.sv", 240))

But Verilator accepts the file.

This may be a known issue. Is it actually even Verilog-compliant to remove the if(1)?

If not, then there is a bug in sv2v, because applying sv2v to prim_flop_2sync.sv removes this if(1).

It would be extra nice if we could let paragraph "anchor" symbols appear that users can click to get the direct link to an element (like on docs.rs). 🙂 I tried adding the HTML code for that, but there seems to be a script or CSS file missing -- any idea which? #17

Hi there!

On some input, Morty seems to panic with

thread 'main' panicked at 'called `Option::unwrap()` on a `None` value', src/lib.rs:92:83
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

You can refer to this repository to reproduce the issue and test your fix.

Thanks!
Flavien

Not directly related, but all those <h3 ... strings are very similar and their construction should be refactored IMO.

Originally posted by @accuminium in #21

Ideally, when able to also process comments, we can automatically parse documentation. Let's start by gathering some ideas here:

• Document module hierarchy
• Markdown comments at top of file, similar to Rust /// and //!.
• Port descriptions

Hello,

morty version 0.9.0 fails with a parse error in the following code example:

// filename: inside_test.sv
module test (bit clk);
  int a,b,c;
  bit d;

    if (a inside {b, c}) begin : gen_1
      assign d = 1'b1;
    end else begin : gen_0
      assign d = 1'b0;
    end
endmodule

Tool was called with morty inside_test.sv.
Error message:

Error: parse error
   --> inside_test.sv:21:11
   |
21 |     if (a inside {b, c}) begin : gen_1
   |           ^
Error: Parse error: Some(("inside_test.sv", 403))

Another test case where we expect no parse error on the 'inside' expression is:

//   typedef enum logic [2:0] {
//     testval1  = 3'd0, 
//     testval2  = 3'd1, 
//     testval3  = 3'd2
//   } test_e;
module test#(parameter test_e a) (bit clk);
  bit d;

    if (a inside {testval1, testval2}) begin : gen_1
      assign d = 1'b1;
    end else begin : gen_0
      assign d = 1'b0;
    end

endmodule

Thank you!

Only re-name modules which are defined in the source file set. Currently, everything gets re-named. We potentially want to make a first pass through the source file set to determine modules and packages which should be re-named.

We are currently using signal as prefix in the ID of all VarItems. I wonder if we should change that to var to be closer to the language.

Originally posted by @accuminium in #21

If we encounter a preprocessor parse error the error handling is below optimal. I think we can have a look at sv-lint to check how parse errors should be handled.

Hi there!

In some cases, $fatal is guarded by synopsys translate_off/on, such as in the code below.

https://github.com/black-parrot/black-parrot/blob/6ce6523d6b3ab91cdb355d53c419f1b9ee1e1e81/bp_common/src/v/bsg_bus_pack.sv#L53-L59

As opposed to pragma translate_off/on, Morty does not seem to ignore snippets guarded by synopsys translate_off/on, and the $fatal results in a parse error.

Some solutions include ignoring snippets guarded by synopsys translate_off/on (may not be backward compatible) or supporting $fatal.

Thanks!
Flavien

Hi there! Me again 😇

Morty stack overflows on some CPU code.

Output

thread '<unknown>' has overflowed its stack
fatal runtime error: stack overflow
[1]    30715 IOT instruction (core dumped)  /home/user/.cargo/bin/morty -f Bender.sources

How to reproduce

Tried on Ubuntu 22.04 with morty 0.8.0.

git clone https://github.com/flaviens/a2o.git
cd a2o
git checkout reprod_morty_stkovf
morty -f Bender.sources -DNCLK_WIDTH=1

Thanks!