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.

Sign up for GitHub

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

Jump to bottom

Make the files consistently formatted #1

Open
mithro opened this issue Jun 26, 2020 · 10 comments
Open

Make the files consistently formatted #1

mithro opened this issue Jun 26, 2020 · 10 comments
Assignees
@hzeller

Comments

@mithro
Copy link
Member

mithro commented Jun 26, 2020

Maybe we can use google/verible for this?
@hzeller
Copy link

hzeller commented Jun 30, 2020

This is how the formatting would look like with current verible; some look better, some worse.
hzeller@1a30641

A lot of files could not be processed, possibly due to some large initializer lists (a known issue with verible)
hzeller@a1758ef

Also one crash on ./verilog/src/unisims/PCIE4CE4.v

CC: @fangism

@fangism
Copy link

fangism commented Jun 30, 2020

Yes, I see a number of bugs that amount to unhandled cases in the formatter's view of the syntax tree. We haven't tested it much on library-style verilog yet, but this makes for a nice sample set.

hzeller added a commit to hzeller/XilinxUnisimLibrary that referenced this issue Aug 19, 2020
@hzeller
Running verible-verilog-formatter 2020-08-19
20a7142 
A few logfiles
 * [process.log](./process.log): exit code. 67 files parsed and formatted,
   181 files created at least one syntax error, and 2 files
   resulted in a segfault.

 * [syntaxerrors.log](./syntaxerrors.log) output of syntax errors.
 * [unique-syntaxerrors.log](./unique-syntaxerrors.log) : set
   of unique syntaxerrors, with count.

Explorations for

SymbiFlow#1
@hzeller
Copy link

hzeller commented Aug 19, 2020

Did another round of formatting:
hzeller@20a7142

findings: 181 files out of 250 have some sort of parse error.

Funny: the Xilinx ascii-art, with a backslash at the end of a comment prompts verible to indent:

hzeller@20a7142#diff-6bd9051af528842b89d6bbf7973ba096

@hzeller
Copy link

hzeller commented Aug 19, 2020

Here is a breakdown of syntax errors seen. I suspect a lot of these are due to preprocessing weirdness.

      1 syntax error, rejected "always" (https://github.com/google/verible).
      1 syntax error, rejected ":" (https://github.com/google/verible).
      2 syntax error, rejected "buf" (https://github.com/google/verible).
      2 syntax error, rejected "endtask" (https://github.com/google/verible).
      4 syntax error, rejected "<" (https://github.com/google/verible).
      5 syntax error, rejected ";" (https://github.com/google/verible).
      6 syntax error, rejected "pulldown" (https://github.com/google/verible).
      7 syntax error, rejected "endmodule" (https://github.com/google/verible).
      9 syntax error, rejected "specify" (https://github.com/google/verible).
     10 syntax error, rejected "endspecify" (https://github.com/google/verible).
     13 syntax error, rejected "else" (https://github.com/google/verible).
     14 syntax error, rejected "`ifdef" (https://github.com/google/verible).
     24 syntax error, rejected "=" (https://github.com/google/verible).
     28 syntax error, rejected "`else" (https://github.com/google/verible).
     66 syntax error, rejected "<=" (https://github.com/google/verible).
    160 syntax error, rejected "$width" (https://github.com/google/verible).
    180 syntax error, rejected "$period" (https://github.com/google/verible).
    328 syntax error, rejected "end" (https://github.com/google/verible).
   4393 syntax error, rejected "(" (https://github.com/google/verible).
  10610 syntax error, rejected "$setuphold" (https://github.com/google/verible).
  12088 syntax error, rejected "assign" (https://github.com/google/verible).

@mithro
Copy link
Member Author

mithro commented Aug 19, 2020

@hzeller - A lot of those sound like the "specify" stuff which Yosys also doesn't like.

@hzeller
Copy link

hzeller commented Sep 10, 2020 •
edited by unfurl-links bot

Current status: formatted files in https://github.com/hzeller/XilinxUnisimLibrary/tree/formatting-experience3 branch (this commit )

165 files parse, 85 still fail (vs. 69 parse and 181 failure last time. Progress).

Unique syntax errors:

      1 syntax error, rejected "always" (https://github.com/google/verible).
      1 syntax error, rejected ":" (https://github.com/google/verible).
      2 syntax error, rejected "buf" (https://github.com/google/verible).
      2 syntax error, rejected "endtask" (https://github.com/google/verible).
      4 syntax error, rejected "<" (https://github.com/google/verible).
      5 syntax error, rejected ";" (https://github.com/google/verible).
      6 syntax error, rejected "pulldown" (https://github.com/google/verible).
      7 syntax error, rejected "endmodule" (https://github.com/google/verible).
      9 syntax error, rejected "specify" (https://github.com/google/verible).
     10 syntax error, rejected "endspecify" (https://github.com/google/verible).
     13 syntax error, rejected "else" (https://github.com/google/verible).
     14 syntax error, rejected "`ifdef" (https://github.com/google/verible).
     24 syntax error, rejected "=" (https://github.com/google/verible).
     28 syntax error, rejected "`else" (https://github.com/google/verible).
     66 syntax error, rejected "<=" (https://github.com/google/verible).
    160 syntax error, rejected "$width" (https://github.com/google/verible).
    180 syntax error, rejected "$period" (https://github.com/google/verible).
    328 syntax error, rejected "end" (https://github.com/google/verible).
   4393 syntax error, rejected "(" (https://github.com/google/verible).
  10610 syntax error, rejected "$setuphold" (https://github.com/google/verible).
  12088 syntax error, rejected "assign" (https://github.com/google/verible).
GitHub
Apache 2.0 licensed copy of the Xilinx Unisim library. - hzeller/XilinxUnisimLibrary

@hzeller
Copy link

hzeller commented Sep 10, 2020

I think a lot of errors also come from limited preprocessing by Verible and subsequent confusion.

@hzeller
Copy link

hzeller commented Sep 30, 2020

New run. Some formatting changes since last time (typically looks better):

hzeller@0ed1d38

145 files parse, 81 fail, and 24 crash formatting (b/169742626). Crashes are regression from last time:

    times | exit code | remark
----------+-----------+--------
      24  |   134     | crashes
      81  |     1     | parse failures
     145  |     0     | parses nicely

@hzeller
Copy link

hzeller commented Sep 30, 2020 •
edited

Most notable changes:

  • (good) A lot of $display("very long stuff", foo, bar) became broken to multiple lines.
  • (good) A lot of improvement in tabular aligning, in particular assignments.

if/else are more compact:

if (foo)
   bar;
else if (baz)
   quux;

became

 if (foo) bar;
 else if (baz) quux;

often this is better, sometimes not necessarily when the if expression is pretty large.

Concatenation operators became worse: (verilog/src/unisims/DSP_ALU.v)

  assign comux_w = ((smux & {comux4simd[46:0], 1'b0}) |
                    (wmux & {comux4simd[46:0], 1'b0}) |
                    (smux & wmux));

became:

  assign comux_w = ((smux & {
    comux4simd[46:0], 1'b0
  }) | (wmux & {
    comux4simd[46:0], 1'b0
  }) | (smux & wmux));

@fangism
Copy link

fangism commented Sep 30, 2020

Most notable changes:

  • (good) A lot of $display("very long stuff", foo, bar) became broken to multiple lines.
  • (good) A lot of improvement in tabular aligning, in particular assignments.

if/else are more compact:

if (foo)
   bar;
else if (baz)
   quux;

became

 if (foo) bar;
 else if (baz) quux;

often this is better, sometimes not necessarily when the if expression is pretty large.

Concatenation operators became worse: (verilog/src/unisims/DSP_ALU.v)

  assign comux_w = ((smux & {comux4simd[46:0], 1'b0}) |
                    (wmux & {comux4simd[46:0], 1'b0}) |
                    (smux & wmux));

became:

  assign comux_w = ((smux & {
    comux4simd[46:0], 1'b0
  }) | (wmux & {
    comux4simd[46:0], 1'b0
  }) | (smux & wmux));

Acknowledged.

New run. Some formatting changes since last time (typically looks better):

hzeller@0ed1d38

145 files parse, 81 fail, and 24 crash formatting (b/169742626). Crashes are regression from last time:

    times | exit code | remark
----------+-----------+--------
      24  |   134     | crashes
      81  |     1     | parse failures
     145  |     0     | parses nicely

Acknowledged. If you have time, issues with reduced test cases would be helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hzeller hzeller

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@mithro @hzeller @fangism