Project

General

Profile

[logo] 
 
Home
News
Activity
About/Contact
Major Tools
  Dinotrace
  Verilator
  Verilog-mode
  Verilog-Perl
Other Tools
  IPC::Locker
  Parallel::Forker
  Voneline
General Info
  Papers

Issue #1656

[RFC] Relaxing UNOPTFLAT by dividing unpacked array

Added by Yutetsu TAKATSUKASA 2 months ago. Updated 2 months ago.

Status:
Assigned
Priority:
Normal
Category:
Performance
% Done:

0%


Description

I have some idea and code to relax UNOPTFLAT limitation. If this looks useful, I'd like to go forward to polish the code.


I guess there are many design patterns that cause UNOPTFLAT warning.

What I recently see is something like below (barrel shifter).
logic [width-1:0] tmp[0:depth];
generate
    for(genvar i = 0; i < depth; ++i) begin
        always_comb
        if (shift[i]) begin
            tmp[i+1] = {tmp[i][(1 << i)-1:0], tmp[i][width-1:(2**i)]};
        end else begin
            tmp[i + 1] = tmp[i];
        end
    end
endgenerate

If tmp is divided in to depth words, verilator won't be confused.

I wrote a PoC of a pass to do it. https://github.com/yTakatsukasa/verilator/commit/a91cbe83fcd8bb7e4f43f074857f03a1f8b4bac7 Because the code is PoC, debug message appears in stdout.

The PoC splits an unpacked array if the array has pragma of /*verilator split_array/* .

I add an example of test_regress/t/t_split_array.{pl,v}. If the pragma is removed from the example, UNOPTFLAT appears.

Here is some my thought on the spec.

  • Each word of unpacked array is independent each other, but not true for packed array. So support only unpacked array.
  • If FLATUNOPT for an array is false positive, changing from packed to unpacked should be easy.
  • It is hard to make robust judgement of which array to split. Inappropriate split explodes verilating time. So user have to specify by pragma.
  • Support just 1D array in the initial version. Enhance it to multi-dimension is doable but less likely to be necessary.

History

#1 Updated by Wilson Snyder 2 months ago

  • Status changed from New to Assigned

Excellent!

I've seen these common cases as causing UNOPTFLAT:
  • Unpacked arrays
  • Packed arrays
  • Unpacked structs
  • Packed structs
  • Bitfields within a signal. (Especially Verilog code predating structs/2D arrays.)

Because Verilator currently converts the last four to bitfields, this is really only two cases to split up. So I really think we need both to be solved; the code should be similar.

Other notes:
  • Suggest using just /*verilator split_var*/ since some cases aren't arrays.
  • Suggest a complete new .h/.cpp for this since seems orthogonal to V3Split.
  • Suggest unoptflat be improved to suggest to users what variables to /*verilator split*/.
    • Long term unoptflat could spit out a .vlt file that marks them.
  • Suggest tests should try the 5 cases mentioned above. (Perhaps can be same test code with ifdefs?)
  • Ok with 1D for now. Should probably error out if user puts split on 2D+ until this is supported.
  • Say we split "a[7:4]" from "a[3:0]", if the code references "a", we need to replace "a" with "CONCAT"
  • Instead of std::cout, use UINFO Then you can leave that code in forever and pass --debugi-V3Split 9 to turn on the messages. (Or another number < 9, but < 4 numbers will turn on with just --debug)

#2 Updated by Yutetsu TAKATSUKASA 2 months ago

Thanks for the suggestions. I agree with the you.

I'll add this feature step by step and ask your review.

  1. 1D unpacked arrays
  2. 2D and above unpacked arrays
  3. other 4 types which are bitfields internally

3 may come before 2 though.

Anyway I will make sure that verilator emits warning/error for split_var pragma for not-yet-supported datatype in any steps.

Also available in: Atom