uwsampl / yosys

Yosys Open SYnthesis Suite
https://yosyshq.net/yosys/
ISC License
1 stars 1 forks source link

Add Lakeroad egglog backend #4

Open gussmith23 opened 1 year ago

thiskappaisgrey commented 12 months ago

How do I add a test to your PR? I found something that doesn't work:

module test 
  (
   i_bit1,
   );

  input  i_bit1;

  assign o_sum   = ~ i_bit1; 

endmodule // half_adder

This is the yosys file:

# read design
read_verilog ./test.v 
write_lakeroad 

I get this error:

; cells
ERROR: Unimplemented cell type $not for cell test.$not$./test.v:10$1.
gussmith23 commented 12 months ago

There are two approaches. In this case, you need a very basic test that just checks to make sure the Lakeroad backend doesn't crash when it runs on a given module. In that case, you should write a test that looks like Yosys's tests. Take a look at backends/lakeroad/example.ys. This test is a Yosys script that reads in a module and runs the backend. If no error is thrown, the test passes. So you could copy this file and change the module.

The second type of test is something that doesn't currently exist in Yosys; this PR adds it. It realistically won't get merged into Yosys itself. But nevertheless, it's useful for doing fine-grained testing -- for example, if you want to make sure that the Lakeroad backend produces SPECIFIC output. For this I use lit and FileCheck, which are LLVM integration testing tools. For an example, look at backends/lakeroad/tests/permuter.sv. The syntax is pretty simple: you have a source file (e.g. a Verilog file). In comments at the top of the file, you give the command to compile the file (in a RUN comment). Then, you write CHECK comments to check that the output is as expected.

thiskappaisgrey commented 12 months ago

How do I add the basic test then? It seems you only have lit tests in the repo for lakeroad.

Edit, just added a PR: #8 - can you check? Thanks.

gussmith23 commented 12 months ago

Look in this PR you're commenting on -- I gave the file path to example.ys added by this PR -- you can copy that file and open a new PR that will merge into this open PR (like what you did on the lakeroad repo a few weeks ago).

More clearly: you should be adding the file into Yosys, not Lakeroad. And you should add it specifically to this PR.

On Wed, Dec 6, 2023, 11:54 AM Thanawat Techaumnuaiwit < @.***> wrote:

How do I add the basic test then? It seems you only have lit tests in the repo for lakeroad.

— Reply to this email directly, view it on GitHub https://github.com/uwsampl/yosys/pull/4#issuecomment-1843594065, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJFZAK2PYVNARDFV24MSK3YIDEQLAVCNFSM6AAAAAA4G52N3SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBTGU4TIMBWGU . You are receiving this because you authored the thread.Message ID: @.***>

thiskappaisgrey commented 12 months ago

I added a test here: https://github.com/uwsampl/yosys/pull/8

On Wed, Dec 6, 2023 at 2:14 PM Gus Smith @.***> wrote:

Look in this PR you're commenting on -- I gave the file path to example.ys added by this PR -- you can copy that file and open a new PR that will merge into this open PR (like what you did on the lakeroad repo a few weeks ago).

More clearly: you should be adding the file into Yosys, not Lakeroad. And you should add it specifically to this PR.

On Wed, Dec 6, 2023, 11:54 AM Thanawat Techaumnuaiwit < @.***> wrote:

How do I add the basic test then? It seems you only have lit tests in the repo for lakeroad.

— Reply to this email directly, view it on GitHub https://github.com/uwsampl/yosys/pull/4#issuecomment-1843594065, or unsubscribe < https://github.com/notifications/unsubscribe-auth/AAJFZAK2PYVNARDFV24MSK3YIDEQLAVCNFSM6AAAAAA4G52N3SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBTGU4TIMBWGU>

. You are receiving this because you authored the thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/uwsampl/yosys/pull/4#issuecomment-1843774659, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEAWXROII7TIP6RC4VCBQ6LYIDU4XAVCNFSM6AAAAAA4G52N3SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBTG43TINRVHE . You are receiving this because you commented.Message ID: @.***>

gussmith23 commented 12 months ago

Great, thank you for this!

Message ID: @.***>

thiskappaisgrey commented 10 months ago

How should we make the egglog backend work for clocked designs? For example, this code:

module ALU_32bit (
  input logic [4:0] operandA,
  input logic [4:0] operandB,
  input logic [3:0] control, // Control signal to specify the operation
  output logic [4:0] result

);

  always  begin
    case (control)
      // 4'b0000: result = operandA + operandB; // Addition
      2'b00: result = operandA + operandB; // Bitwise XOR
      2'b01: result = operandA - operandB; // Subtraction
      2'b10: result = operandA | operandB; // Bitwise OR
      2'b11: result = operandA & operandB; // Bitwise AND
    endcase
  end
endmodule

implies a clocked design (using a global clock) and this needs the flip-flop cells to be implemented. Maybe I can implement it using assign (which .. maybe doesn't use a mux?? I have no clue on the semantics of verilog).. but for more complex tests, I'll need the flipflops anyways... Maybe there's a way to expand the flip-flops into gates with yosys? If there is, please let me know. Thanks.

gussmith23 commented 10 months ago

Two things:

  1. I'm not understanding what you mean when you say that that design "implies a clocked design". The module is purely combinational -- there's no register/flip flop (explicitly OR implicitly) in the design!
  2. Clocked designs should already work out of the box! See the following example of a permuter with a register on its output: https://github.com/uwsampl/yosys/pull/4/files#diff-df8bc67a92c0805db2e06e9acdb0f35ae305dc5b7ac039d4647a343c4491a809

It used to be that Regs explicitly took a clock input; now they assume a global clock. I may switch that back later though; I don't really understand the benefits of an implicit global clock (other than terseness, which I don't believe is something intermediate representations should care about).

By the way -- we're finishing up ASPLOS camera ready today. After that I will become much more free. I'm gonna send you/Jon/Zach an email about potentially setting up recurring meetings.

On Sun, Jan 7, 2024 at 9:24 PM Thanawat Techaumnuaiwit < @.***> wrote:

How should we make the egglog backend work for clocked designs? For example, this code:

module ALU_32bit ( input logic [4:0] operandA, input logic [4:0] operandB, input logic [3:0] control, // Control signal to specify the operation output logic [4:0] result

);

always begin case (control) // 4'b0000: result = operandA + operandB; // Addition 2'b00: result = operandA + operandB; // Bitwise XOR 2'b01: result = operandA - operandB; // Subtraction 2'b10: result = operandA | operandB; // Bitwise OR 2'b11: result = operandA & operandB; // Bitwise AND endcase end endmodule

implies a clocked design (using a global clock) and this needs the flip-flop cells to be implemented. Maybe I can implement it using assign (which .. maybe doesn't use a mux?? I have no clue on the semantics of verilog).. but for more complex tests, I'll need the flipflops anyways... Maybe there's a way to expand the flip-flops into gates with yosys? If there is, please let me know. Thanks.

— Reply to this email directly, view it on GitHub https://github.com/uwsampl/yosys/pull/4#issuecomment-1880402073, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJFZAN5Y6V2JTCH6DE35FTYNN7HTAVCNFSM6AAAAAA4G52N3SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBQGQYDEMBXGM . You are receiving this because you authored the thread.Message ID: @.***>

thiskappaisgrey commented 10 months ago

Oh, when I compile it, it adds an $ff yosys cell and in the lake road backend, that (and the other flip flop cells) are not implemented. Idk how to get rid of the flip-flop cells and compile it to other logic. That's what I meant. I meant for the design to be purely combinatorial but the case statement can only be used in an always block, and the compiled yosys code includes an always @* (or something like that), so I'm assuming that's clocked towards a global clock or something idk.. I'll try messing around with the yosys statements later and see.. recurring meetings would be nice! Just lmk.

gussmith23 commented 10 months ago

Can you post the script you're using to compile it? Specifically, what Yosys passes are you running before you run the Lakeroad backend?

thiskappaisgrey commented 10 months ago

Oh, I can do that when I get the chance! On mobile right now.

thiskappaisgrey commented 10 months ago

I never figured out how to get the case statement working but I'll add a not-working test case in a branch later. I managed to get it working by using a ternary (the always block adds flip-flops for some reason).

gussmith23 commented 10 months ago

Try always_comb instead of always. You should at least get a more informative error.

It's likely because of case. You think you're handling all cases, but you're not -- what if one or both bits is X or Z? Related to this, you could also just try adding a default: case to your case block. That would probably resolve it.

gussmith23 commented 10 months ago

That is:

      2'b11: result = operandA & operandB; // Bitwise AND
      default: result = 0;    
    endcase
thiskappaisgrey commented 10 months ago

Oh yeah - the verilog experts in my lab helped resolve the issue. Thanks!