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

duplicated ranges #521

Merged
merged 4 commits into from
Aug 19, 2023
Merged

duplicated ranges #521

merged 4 commits into from
Aug 19, 2023

Conversation

wsipak
Copy link
Collaborator

@wsipak wsipak commented May 29, 2023

  1. This refactors add_multirange_attribute function.
  • The purpose of the function is to put data about multiranges in node->multirange_dimensions vector, and as it doesn't do anything related to node->attributes, the name could be considered misleading. I'm renaming it to set_multirange_dimensions and will be happy to consider other options, too.
  • There was a node duplicated and then both nodes were simplified. Now it's duplicated after simplification.
  • Ideally, the function should be called only once for each node. The function will now use both packed_ranges and unpacked_ranges in a single call, so that it's easier to forbid multiple calls of the function later on.
  1. The process_net function has a bug which causes ranges to be duplicated. With the current solution, the same range is added to a node here: https://github.com/chipsalliance/yosys-f4pga-plugins/blob/main/systemverilog-plugin/UhdmAst.cc#L501 and here: https://github.com/chipsalliance/yosys-f4pga-plugins/blob/main/systemverilog-plugin/UhdmAst.cc#L542
    The solution is that during the part when process_* functions are executed, we should store information about the ranges in attributes, and later on in the simplification process replace the attributes with children nodes.

Input:

typedef logic [3:0][7:0] my_struct_packed_t;
module single_range_dot_access (
    input  my_struct_packed_t x,
    output my_struct_packed_t y
);
   assign y[2][1] = x[0][0];
endmodule : single_range_dot_access

Result without the changes:

    module single_range_dot_access(x, y);
      (* wiretype = "\my_struct_packed_t" *)
      input [1023:0] x;
      (* wiretype = "\my_struct_packed_t" *)
      output [1023:0] y;
      assign y[319:288] = x[31:0];
      /** AST_TYPEDEF **/
    endmodule

Result with the changes and Surelog 654c4fe230 and 7689edf1:

module single_range_dot_access(x, y);
  (* wiretype = "\my_struct_packed_t" *)
  input [31:0] x;
  (* wiretype = "\my_struct_packed_t" *)
  output [31:0] y;
  assign y[9:9] = x[0:0];
  /** AST_TYPEDEF **/
endmodule

The size of the input/output wires is fixed.
With the current Surelog (bf13c83) it doesn't appear to break anything, and also fixes one FV test.

CI with the changes:
https://github.com/antmicro/yosys-systemverilog/actions/runs/5309555386
https://github.com/antmicro/yosys-systemverilog/actions/runs/5290316276

@wsipak wsipak force-pushed the wsip/duplicated_ranges branch 4 times, most recently from e3e5793 to 260df98 Compare June 16, 2023 12:06
@wsipak wsipak marked this pull request as ready for review June 16, 2023 13:11
@wsipak wsipak requested review from mglb, kamilrakoczy and mandrys and removed request for mglb and kamilrakoczy June 16, 2023 14:53
systemverilog-plugin/UhdmAst.cc Outdated Show resolved Hide resolved
systemverilog-plugin/UhdmAst.cc Show resolved Hide resolved
@mglb mglb merged commit 17519a6 into chipsalliance:main Aug 19, 2023
13 checks passed
@mglb mglb deleted the wsip/duplicated_ranges branch August 19, 2023 19:04
mglb pushed a commit to chipsalliance/synlig that referenced this pull request Aug 22, 2023
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

Successfully merging this pull request may close these issues.

3 participants