Don't warn on BLKSEQ

flake.nix

@@ -12,7 +12,7 @@
           verilator = import ./verilator.nix pkgs ;
           yosys = pkgs.yosys;
 
-          vflags = ''-Wpedantic -Wwarn-lint -Wwarn-style -Wno-PINCONNECTEMPTY -CFLAGS "-Wpedantic -std=c++20"'';
+          vflags = ''-Wpedantic -Wwarn-lint -Wwarn-style -Wno-PINCONNECTEMPTY -Wno-BLKSEQ -CFLAGS "-Wpedantic -std=c++20"'';
 
           verilate-src = cmd: ''
             cp -r ${./src} ./src
@@ -32,6 +32,15 @@
             mkdir "$out/bin" && cp "$out/Valu" "$out/bin/alu-sim"
           '';
 
+          alu-sim-trace = pkgs.runCommandCC "alu-sim-trace" {} ''
+            ${verilate-src "--cc --build --exe --trace -CFLAGS -DTRACE=1 ./simulation/tester.cpp ./simulation/test_alu.cpp -top alu"}
+            mv obj_dir "$out"
+            mkdir "$out/bin" && cp "$out/Valu" "$out/bin/alu-sim"
+            $out/bin/alu-sim $out/trace.vcd
+            echo "${pkgs.gtkwave}/bin/gtkwave $out/trace.vcd" > $out/bin/alu-sim-trace
+            chmod u+x $out/bin/alu-sim-trace
+          '';
+
           alu-synth = pkgs.runCommandCC "alu-synth" {} ''
             mkdir -p "$out"
             find ${./src} -name '*.v' -exec ${yosys}/bin/yosys -Q -p "synth_ice40 -top topmost -json $out/synth.json -dsp" {} +
@@ -42,7 +51,7 @@
           '';
 
           deps = [
-            yosys pkgs.nextpnrWithGui pkgs.icestorm verilator
+            yosys pkgs.nextpnrWithGui pkgs.icestorm verilator pkgs.gtkwave
           ];
       in rec {
         packages.verilator = verilator;
@@ -50,6 +59,8 @@
         packages.lint = lint;
 
         packages.alu-sim = alu-sim;
+        packages.alu-sim-trace = alu-sim-trace;
+
         packages.alu-synth = alu-synth;
         packages.alu-synth-view = alu-synth-view;
 

simulation/test_alu.cpp

@@ -3,9 +3,24 @@
 #include "tester.hpp"
 
 #include <stdint.h>
+#include <iostream>
 #include <random>
 
+#ifdef TRACE
+#include "verilated_vcd_c.h"
+#endif
+
+struct state {
+    VerilatedContext *ctx;
+    Valu *valu;
+    #ifdef TRACE
+    VerilatedVcdC *trace;
+    #endif
+};
+
 struct alu_testcase {
+    state *state;
+
     std::string name;
     // Inputs
     uint32_t A, B;
@@ -30,77 +45,156 @@ std::string fmt_hex(uint32_t n) {
     return hex;
 }
 
-void test_op(Valu *valu, Tester *tester, alu_testcase test) {
+void posedge(state *state) {
+    #ifdef TRACE
+        state->ctx->timeInc(1);
+        state->valu->CLK = 1;
+        state->valu->eval();
+        state->trace->dump(state->ctx->time());
+
+        state->ctx->timeInc(1);
+        state->valu->CLK = 0;
+        state->valu->eval();
+        state->trace->dump(state->ctx->time());
+    #else
+        state->ctx->timeInc(1);
+        state->valu->CLK = 1;
+        state->valu->eval();
+
+        state->ctx->timeInc(1);
+        state->valu->CLK = 0;
+        state->valu->eval();
+    #endif
+}
+
+void test_op(Tester *tester, alu_testcase test) {
     Tester subtester(tester, test.name);
 
-    // assign inputs
-    valu->op = test.op;
-    valu->A = test.A;
-    valu->B = test.B;
+    posedge(test.state);
 
-    valu->eval();
+    // assign inputs
+    test.state->valu->op = test.op;
+    test.state->valu->A = test.A;
+    test.state->valu->B = test.B;
+    test.state->valu->EN = 1;
+    posedge(test.state);
+    test.state->valu->EN = 0;
+
+    int max_cycles = test.max_cycles.has_value() ? *test.max_cycles : 10000;
+
+    int n_cycles = 1;
+    for (; !test.state->valu->RDY && n_cycles < 10 + max_cycles * 2; n_cycles++) {
+        posedge(test.state);
+    }
+
+    char rdy_after_s[100];
+    snprintf(rdy_after_s, sizeof rdy_after_s, "RDY = 1 (after %d cycle(s))", n_cycles);
+    subtester.assert_eq(rdy_after_s, test.state->valu->RDY, 1);
+
+    if (test.max_cycles.has_value()) {
+        if (n_cycles <= test.max_cycles) {
+            char n_cycles_s[100];
+            snprintf(n_cycles_s, sizeof n_cycles_s, "Finished within %d cycle(s) (actually %d)", test.max_cycles, n_cycles);
+            subtester.assert_eq(n_cycles_s, n_cycles, n_cycles);
+        } else {
+            subtester.assert_eq("Finished within correct number of cycles", n_cycles, test.max_cycles);
+        }
+    }
 
     std::string o_name("O == ");
     o_name.append(fmt_hex(test.O));
 
-    subtester.assert_eq(o_name, valu->O, test.O);
+    subtester.assert_eq(o_name, test.state->valu->O, test.O);
 
     if (test.overflow.has_value()) {
         if (*test.overflow)
-            subtester.assert_eq("overflow flag set", valu->Fflow, 1);
+            subtester.assert_eq("overflow flag set", test.state->valu->Fflow, 1);
         else
-            subtester.assert_eq("no overflow flag", valu->Fflow, 0);
+            subtester.assert_eq("no overflow flag", test.state->valu->Fflow, 0);
     }
     if (test.zero.has_value()) {
         if (*test.zero)
-            subtester.assert_eq("zero flag set", valu->Fzero, 1);
+            subtester.assert_eq("zero flag set", test.state->valu->Fzero, 1);
         else
-            subtester.assert_eq("no zero flag", valu->Fzero, 0);
+            subtester.assert_eq("no zero flag", test.state->valu->Fzero, 0);
     }
 }
 
 int main(int argc, char **argv) {
+    bool DO_AUTO = false;
+
     VerilatedContext *vctx = new VerilatedContext;
+    Verilated::traceEverOn(true);
     Valu *valu = new Valu(vctx);
 
+    #ifdef TRACE
+        if (argc != 2) {
+            std::cout << "Run with argument for destination!" << std::endl;
+            return 1;
+        }
+
+        VerilatedVcdC *trace = new VerilatedVcdC;
+        valu->trace(trace, 99);
+        trace->open(argv[1]);
+        std::cout << "(writing trace to " << argv[1] << ")" << std::endl;
+
+        state state = {
+            .ctx = vctx,
+            .valu = valu,
+            .trace = trace,
+        };
+    #else
+        state state = {
+            .ctx = vctx,
+            .valu = valu,
+        };
+    #endif
+
     Tester alu_t("alu");
     {
         Tester add_t(&alu_t, "add", true);
 
-        test_op(valu, &add_t, {
+        test_op(&add_t, {
+            .state = &state,
             .name = "0x2137+0x1234",
             .A = 0x2137, .B = 0x1234, .op = 0b000,
             .O = 0x336b, .overflow = false,
         });
 
-        test_op(valu, &add_t, {
+        test_op(&add_t, {
+            .state = &state,
             .name = "0x09+0x10",
             .A = 0x09, .B = 0x10, .op = 0b000,
             .O = 0x19, .overflow = false,
         });
 
-        test_op(valu, &add_t, {
+        test_op(&add_t, {
+            .state = &state,
             .name = "0x5555+0x5555",
             .A = 0x5555, .B = 0x5555, .op = 0b000,
             .O = 0xaaaa, .overflow = false,
         });
 
-        test_op(valu, &add_t, {
+        test_op(&add_t, {
+            .state = &state,
             .name = "0xfffffffe+0x1",
             .A = 0xfffffffe, .B = 0x1, .op = 0b000,
             .O = 0xffffffff, .overflow = false, .zero = false,
         });
-        test_op(valu, &add_t, {
+        test_op(&add_t, {
+            .state = &state,
             .name = "0xffffffff+0x1",
             .A = 0xffffffff, .B = 0x1, .op = 0b000,
             .O = 0x0, .overflow = true, .zero = true,
         });
-        test_op(valu, &add_t, {
+        test_op(&add_t, {
+            .state = &state,
             .name = "0xffffffff+0x2",
             .A = 0xffffffff, .B = 0x2, .op = 0b000,
             .O = 0x1, .overflow = true, .zero = false,
         });
-        test_op(valu, &add_t, {
+        test_op(&add_t, {
+            .state = &state,
             .name = "0x0+0x0",
             .A = 0x0, .B = 0x0, .op = 0b000,
             .O = 0x0, .overflow = false, .zero = true,
@@ -110,31 +204,36 @@ int main(int argc, char **argv) {
     {
         Tester sub_t(&alu_t, "sub", true);
 
-        test_op(valu, &sub_t, {
+        test_op(&sub_t, {
+            .state = &state,
             .name = "0x2137-0x0420",
             .A = 0x2137, .B = 0x0420, .op = 0b001,
             .O = 0x1d17, .overflow = false,
         });
 
-        test_op(valu, &sub_t, {
+        test_op(&sub_t, {
+            .state = &state,
             .name = "0x0-0x1",
             .A = 0x0, .B = 0x1, .op = 0b001,
             .O = 0xffffffff, .overflow = true,
         });
 
-        test_op(valu, &sub_t, {
+        test_op(&sub_t, {
+            .state = &state,
             .name = "0x100-0x0200",
             .A = 0x100, .B = 0x200, .op = 0b001,
             .O = 0xffffff00, .overflow = true,
         });
 
-        test_op(valu, &sub_t, {
+        test_op(&sub_t, {
+            .state = &state,
             .name = "0x21-0x9",
             .A = 0x21, .B = 0x9, .op = 0b001,
             .O = 0x18, .overflow = false, .zero = false,
         });
 
-        test_op(valu, &sub_t, {
+        test_op(&sub_t, {
+            .state = &state,
             .name = "0x20-0x20",
             .A = 0x20, .B = 0x20, .op = 0b001,
             .O = 0x0, .overflow = false, .zero = true,
@@ -145,38 +244,43 @@ int main(int argc, char **argv) {
         Tester bitwise_t(&alu_t, "bitwise", true);
         // 0x3 = 0b0011, 0x5 = 0b0101
 
-        test_op(valu, &bitwise_t, {
+        test_op(&bitwise_t, {
+            .state = &state,
             .name = "0x3&0x5",
             .A = 0x3, .B = 0x5, .op = 0b100,
             .O = 0x1, .zero = false,
         });
 
-        test_op(valu, &bitwise_t, {
+        test_op(&bitwise_t, {
+            .state = &state,
             .name = "0x3|0x5",
             .A = 0x3, .B = 0x5, .op = 0b101,
             .O = 0x7, .zero = false,
         });
 
-        test_op(valu, &bitwise_t, {
+        test_op(&bitwise_t, {
+            .state = &state,
             .name = "0x3^0x5",
             .A = 0x3, .B = 0x5, .op = 0b110,
             .O = 0x6, .zero = false,
         });
 
-        test_op(valu, &bitwise_t, {
+        test_op(&bitwise_t, {
+            .state = &state,
             .name = "~0xa5a5a5a5",
             .A = 0xa5a5a5a5, .B = 0x0, .op = 0b111,
             .O = 0x5a5a5a5a, .zero = false,
         });
 
-        test_op(valu, &bitwise_t, {
+        test_op(&bitwise_t, {
+            .state = &state,
             .name = "~0xffffffff",
             .A = 0xffffffff, .B = 0x0, .op = 0b111,
             .O = 0x0, .zero = true,
         });
     }
 
-    {
+    if (DO_AUTO) {
         Tester auto_t(&alu_t, "auto", true);
 
         std::default_random_engine eng;
@@ -194,7 +298,8 @@ int main(int argc, char **argv) {
                 name.append("+");
                 name.append(fmt_hex(B));
 
-                test_op(valu, &auto_t, {
+                test_op(&auto_t, {
+                    .state = &state,
                     .name = name,
                     .A = A, .B = B, .op = 0b000,
                     .O = A + B, .overflow = (A + B < A), .zero = (A + B == 0),
@@ -206,7 +311,8 @@ int main(int argc, char **argv) {
                 name.append("-");
                 name.append(fmt_hex(B));
 
-                test_op(valu, &auto_t, {
+                test_op(&auto_t, {
+                    .state = &state,
                     .name = name,
                     .A = A, .B = B, .op = 0b001,
                     .O = A - B, .overflow = (B > A), .zero = (A == B),
@@ -218,7 +324,8 @@ int main(int argc, char **argv) {
                 name.append("&");
                 name.append(fmt_hex(B));
 
-                test_op(valu, &auto_t, {
+                test_op(&auto_t, {
+                    .state = &state,
                     .name = name,
                     .A = A, .B = B, .op = 0b100,
                     .O = A & B, .overflow = 0, .zero = ((A & B) == 0),
@@ -230,7 +337,8 @@ int main(int argc, char **argv) {
                 name.append("|");
                 name.append(fmt_hex(B));
 
-                test_op(valu, &auto_t, {
+                test_op(&auto_t, {
+                    .state = &state,
                     .name = name,
                     .A = A, .B = B, .op = 0b101,
                     .O = A | B, .overflow = 0, .zero = ((A | B) == 0),
@@ -242,7 +350,8 @@ int main(int argc, char **argv) {
                 name.append("^");
                 name.append(fmt_hex(B));
 
-                test_op(valu, &auto_t, {
+                test_op(&auto_t, {
+                    .state = &state,
                     .name = name,
                     .A = A, .B = B, .op = 0b110,
                     .O = A ^ B, .overflow = 0, .zero = ((A ^ B) == 0),
@@ -253,7 +362,8 @@ int main(int argc, char **argv) {
                 name.append("~");
                 name.append(fmt_hex(A));
 
-                test_op(valu, &auto_t, {
+                test_op(&auto_t, {
+                    .state = &state,
                     .name = name,
                     .A = A, .B = B, .op = 0b111,
                     .O = ~A, .overflow = 0, .zero = (A == 0xffffffff),
@@ -263,4 +373,7 @@ int main(int argc, char **argv) {
             }
         }
     }
+    #ifdef TRACE
+    state.trace->close();
+    #endif
 }

src/alu/alu.v

@@ -12,44 +12,99 @@
     111 = not A
 */
 module alu(
+  input CLK,
+  input EN,
+  output reg RDY,
+
   input [31:0] A,
   input [31:0] B,
   input [2:0] op,
-  output [31:0] O,
-  output Fflow,
-  output Fzero
+  output reg [31:0] O,
+  output reg Fflow,
+  output reg Fzero
 );
 
-wire [31:0] adder_out;
+// Constants
+localparam OP_ADD = 3'b000;
+localparam OP_SUB = 3'b001;
+localparam OP_AND = 3'b100;
+localparam OP_OR  = 3'b101;
+localparam OP_XOR = 3'b110;
+localparam OP_NOT = 3'b111;
 
-begin : addsub
-  wire addition = op == 3'b000;
-  wire subtraction = op == 3'b001;
-  wire [31:0] adder_B = subtraction ? ~B : B;
-  wire adder_cout;
+localparam ST_IDLE = 0;
 
-  carry_select_adder a(A, adder_B, subtraction, adder_out, adder_cout);
-  assign Fflow =
-    addition ? adder_cout :
-    subtraction ? ~adder_cout :
-    0;
-end
+// State
 
-wire [31:0] mult_out_hi;
-wire [31:0] mult_out_lo;
-multiplier mult(A, B, mult_out_hi, mult_out_lo);
+reg [8:0] state = ST_IDLE;
+reg [2:0] selected_out;
+
+assign RDY = state == ST_IDLE;
+
+reg [31:0] bitwise_out;
+
+// Outputs
 
 assign O =
-  op == 3'b000 ? adder_out :
-  op == 3'b001 ? adder_out :
-  op == 3'b010 ? mult_out_hi :
-  op == 3'b011 ? mult_out_lo :
-  op == 3'b100 ? A & B :
-  op == 3'b101 ? A | B :
-  op == 3'b110 ? A ^ B :
-  op == 3'b111 ? ~A :
-  '0;
+  (selected_out == OP_ADD || selected_out == OP_SUB) ? adder_out :
+  bitwise_out;
 
-assign Fzero = ~ (| O);
+assign Fflow =
+  selected_out == OP_ADD ? adder_carry_out :
+  selected_out == OP_SUB ? ~adder_carry_out :
+  0;
+
+assign Fzero = ~(| O);
+
+// Modules
+
+reg [31:0] adder_A, adder_B;
+reg adder_carry_in, adder_carry_out;
+wire [31:0] adder_out;
+
+carry_select_adder adder(adder_A, adder_B, adder_carry_in, adder_out, adder_carry_out);
+
+// Clocking
+
+always @(posedge CLK) begin
+  case (state)
+    ST_IDLE: begin
+      if (EN) begin
+        case (op)
+          OP_ADD: begin
+            adder_A <= A;
+            adder_B <= B;
+            adder_carry_in <= 0;
+            selected_out <= OP_ADD;
+          end
+          OP_SUB: begin
+            adder_A <= A;
+            adder_B <= ~B;
+            adder_carry_in <= 1;
+            selected_out <= OP_SUB;
+          end
+          OP_AND: begin
+            bitwise_out <= A & B;
+            selected_out <= OP_AND;
+          end
+          OP_OR: begin
+            bitwise_out <= A | B;
+            selected_out <= OP_OR;
+          end
+          OP_XOR: begin
+            bitwise_out <= A ^ B;
+            selected_out <= OP_XOR;
+          end
+          OP_NOT: begin
+            bitwise_out <= ~A;
+            selected_out <= OP_NOT;
+          end
+          default: begin end // TODO: this should be $stop in verilator, no-op in synthesis
+        endcase
+      end
+    end
+    default: $stop;
+  endcase
+end
 
 endmodule