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

[feature request] support for field name preservation when struct conversion #253

Open
Dragon-Git opened this issue Aug 28, 2023 · 6 comments

Comments

@Dragon-Git
Copy link

Sv2v version 0.11.0 is represented by vectors when converting the structure, and the name of the field cannot be seen in the converted Verilog code, resulting in poor readability, can the name information of the structure be retained in the converted Verilog code?
example:

typedef struct {
  logic [31:0] a;
  logic [31:0] b;
  logic op;
} data_struct_t;

module src (
  output data_struct_t my_data 
  );
  assign my_data.a = 32'h12345678;
  assign my_data.b = 32'h87654321;
  assign my_data.op = 1'b1;
endmodule

module dest (
  input data_struct_t my_data,
  output logic result
  );
  assign result = my_data.op? my_data.a - my_data.b : my_data.b - my_data.a;
endmodule

module top;
  data_struct_t my_data;

  src src_inst (
    .my_data(my_data)
  );

  dest dest_inst (
    .my_data(my_data)
  );

endmodule

Now convert to:

module src (my_data);
        output wire [64:0] my_data;
        assign my_data[64-:32] = 32'h12345678;
        assign my_data[32-:32] = 32'h87654321;
        assign my_data[0] = 1'b1;
endmodule
module dest (
        my_data,
        result
);
        input wire [64:0] my_data;
        output wire result;
        assign result = (my_data[0] ? my_data[64-:32] - my_data[32-:32] : my_data[32-:32] - my_data[64-:32]);
endmodule
module top;
        wire [64:0] my_data;
        src src_inst(.my_data(my_data));
        dest dest_inst(.my_data(my_data));
endmodule

Expected:

module src (my_data);
        output wire [64:0] my_data;
        assign my_data_a = 32'h12345678;
        assign my_data_b = 32'h87654321;
        assign my_data_op = 1'b1;
endmodule
module dest (
        my_data,
        result
);
        input wire [64:0] my_data;
        output wire result;
        assign result = (my_data_op ? my_data_a - my_data_b : my_data_b - my_data_a);
endmodule
module top;
        wire [64:0] my_data;
        src src_inst(.my_data(my_data));
        dest dest_inst(.my_data(my_data));
endmodule
@zachjs
Copy link
Owner

zachjs commented Aug 29, 2023

Thanks for filing this request! I agree that sv2v's struct conversion doesn't produce highly readable output. Many of sv2v's conversions exhibit this limitation to some extent. sv2v was envisioned as part of synthesis flows, producing generated output that didn't necessarily need to be readable. Can you help me understand your use case here? What do you intend to do with the converted output? Is there a pain point in your existing workflow, e.g., #194?

@Dragon-Git
Copy link
Author

For debugging purposes, it is more convenient to have the field names preserved, especially when dealing with very large structs.

I use peakrdl-regblock to convert systemrdl to register systemverilog files, currently it can be synthesized using Design Compiler, but the readability of the synthesized netlist is lower than that of the netlist synthesized based on verilog files, which affects debugging efficiency during gate-level simulation, so I wanted to use sv2v as a pre-synthesis tool to enhance the readability of the netlist.

sv2v was envisioned as part of synthesis flows, producing generated output that didn't necessarily need to be readable.

While I agree with the fact that sv2v was primarily designed for synthesis flows and readability might not be a priority, supporting the preservation of field names would greatly enhance the functionality of sv2v in this context.

@KatCe
Copy link

KatCe commented Sep 18, 2023

Hello. We could also need this feature for debugging purposes. We use sv2v in order to get a Verilog version of a design. Then we feed it into our Yosys-based flow, where we perform further analysis passes. Manually debugging the transformed design (in a wave form viewer) is quite time consuming when the names are not preserved. Would appreciate it. Thank you!

@sifferman
Copy link
Contributor

I wanted to recommend that if this feature is added, I think it should be added only as an option. Arrays of structs are currently synthesized as a BRAM. But if an array of structs is separated into multiple different arrays, then extra BRAMs may be inferred.

Example:

struct {
  logic a,
  logic b
} s[8];
reg [1:0] s [0:7];
// or
reg s_a [0:7];
reg s_b [0:7];

@KatCe
Copy link

KatCe commented Oct 25, 2023

Implementing it as an option sounds good. I also have another use case now, where I would need this feature for generating verification code.

@gggmmm
Copy link

gggmmm commented Nov 26, 2023

I wanted to recommend that if this feature is added, I think it should be added only as an option. Arrays of structs are currently synthesized as a BRAM. But if an array of structs is separated into multiple different arrays, then extra BRAMs may be inferred.

Example:

struct {
  logic a,
  logic b
} s[8];
reg [1:0] s [0:7];
// or
reg s_a [0:7];
reg s_b [0:7];

Then perhaps a pragma before the code you want not to be touched would be best. That way you get other struct nicely maintained name-wise, and the one that you want as BRAM are synthesized as such. The suggestion of a general flag to disable it completely would still be valid with the pragma.

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

5 participants