Update documentation and remove bad solutions

Author: adam-maj

README.md

@@ -242,3 +242,8 @@ tiny-gpu is setup to simulate the execution of both of the above kernels using `
 Running `make test_matadd` or `make test_matmul` will run the specified kernel and output a log file with the complete execution trace of the kernel from start to finish, as well as the intial and final states of data memory.
 
 The `matadd` kernel adds 2 1x8 matrices across 8 threads running on 2 cores, and the `matmul` kernel multiplies 2 2x2 matrices across 4 threads.
+
+# Notes
+
+- Many things that could be wires are registers to make things explicitly synchronous and for code simplicity and clarity.
+- State management does some things in many cycles that could be done in 1 to make control flow explicit.

src/alu.sv

@@ -9,7 +9,7 @@
 module alu (
     input wire clk,
     input wire reset,
-    input wire enable,
+    input wire enable, // If current block has less threads then block size, some ALUs will be inactive
 
     input reg [2:0] core_state,
 
@@ -35,9 +35,10 @@ module alu (
             // Calculate alu_out when core_state = EXECUTE
             if (core_state == 3'b101) begin 
                 if (decoded_alu_output_mux == 1) begin 
-                    // Set values to compare with NZP register
+                    // Set values to compare with NZP register in alu_out[2:0]
                     alu_out_reg <= {5'b0, (rs - rt > 0), (rs - rt == 0), (rs - rt < 0)};
                 end else begin 
+                    // Execute the specified arithmetic instruction
                     case (decoded_alu_arithmetic_mux)
                         ADD: begin 
                             alu_out_reg <= rs + rt;

src/controller.sv

@@ -8,30 +8,28 @@
 module controller #(
     parameter ADDR_BITS = 8,
     parameter DATA_BITS = 16,
-    parameter NUM_CONSUMERS = 4,
-    parameter NUM_CHANNELS = 1,
-    parameter WRITE_ENABLE = 1
+    parameter NUM_CONSUMERS = 4, // The number of consumers accessing memory through this controller
+    parameter NUM_CHANNELS = 1,  // The number of concurrent channels available to send requests to global memory
+    parameter WRITE_ENABLE = 1   // Whether this memory controller can write to memory (program memory is read-only)
 ) (
     input wire clk,
     input wire reset,
 
-    // LSU Interface
+    // Consumer Interface (Fetchers / LSUs)
     input reg [NUM_CONSUMERS-1:0] consumer_read_valid,
     input reg [ADDR_BITS-1:0] consumer_read_address [NUM_CONSUMERS-1:0],
     output reg [NUM_CONSUMERS-1:0] consumer_read_ready,
     output reg [DATA_BITS-1:0] consumer_read_data [NUM_CONSUMERS-1:0],
-
     input reg [NUM_CONSUMERS-1:0] consumer_write_valid,
     input reg [ADDR_BITS-1:0] consumer_write_address [NUM_CONSUMERS-1:0],
     input reg [DATA_BITS-1:0] consumer_write_data [NUM_CONSUMERS-1:0],
     output reg [NUM_CONSUMERS-1:0] consumer_write_ready,
 
-    // Memory Interface
+    // Memory Interface (Data / Program)
     output reg [NUM_CHANNELS-1:0] mem_read_valid,
     output reg [ADDR_BITS-1:0] mem_read_address [NUM_CHANNELS-1:0],
     input reg [NUM_CHANNELS-1:0] mem_read_ready,
     input reg [DATA_BITS-1:0] mem_read_data [NUM_CHANNELS-1:0],
-
     output reg [NUM_CHANNELS-1:0] mem_write_valid,
     output reg [ADDR_BITS-1:0] mem_write_address [NUM_CHANNELS-1:0],
     output reg [DATA_BITS-1:0] mem_write_data [NUM_CHANNELS-1:0],
@@ -43,11 +41,11 @@ module controller #(
         READ_RELAYING = 3'b100,
         WRITE_RELAYING = 3'b101;
 
+    // Keep track of state for each channel and which jobs each channel is handling
     reg [2:0] controller_state [NUM_CHANNELS-1:0];
-    reg [$clog2(NUM_CONSUMERS)-1:0] current_consumer [NUM_CHANNELS-1:0];
-    reg [NUM_CONSUMERS-1:0] channel_serving_consumer;
+    reg [$clog2(NUM_CONSUMERS)-1:0] current_consumer [NUM_CHANNELS-1:0]; // Which consumer is each channel currently serving
+    reg [NUM_CONSUMERS-1:0] channel_serving_consumer; // Which channels are being served? Prevents many workers from picking up the same request.
 
-    // TODO: read/write should be separate channels, and should handle multi-channel
     always @(posedge clk) begin
         if (reset) begin 
             mem_read_valid <= 0;
@@ -63,11 +61,14 @@ module controller #(
 
             current_consumer <= 0;
             controller_state <= 0;
-            channel_serving_consumer <= 0;
+
+            channel_serving_consumer = 0;
         end else begin 
+            // For each channel, we handle processing concurrently
             for (int i = 0; i < NUM_CHANNELS; i = i + 1) begin 
                 case (controller_state[i])
                     IDLE: begin
+                        // While this channel is idle, cycle through consumers looking for one with a pending request
                         for (int j = 0; j < NUM_CONSUMERS; j = j + 1) begin 
                             if (consumer_read_valid[j] && !channel_serving_consumer[j]) begin 
                                 channel_serving_consumer[j] = 1;
@@ -77,6 +78,7 @@ module controller #(
                                 mem_read_address[i] <= consumer_read_address[j];
                                 controller_state[i] <= READ_WAITING;
 
+                                // Once we find a pending request, pick it up with this channel and stop looking for requests
                                 break;
                             end else if (consumer_write_valid[j] && !channel_serving_consumer[j]) begin 
                                 channel_serving_consumer[j] = 1;
@@ -87,6 +89,7 @@ module controller #(
                                 mem_write_data[i] <= consumer_write_data[j];
                                 controller_state[i] <= WRITE_WAITING;
 
+                                // Once we find a pending request, pick it up with this channel and stop looking for requests
                                 break;
                             end
                         end
@@ -108,17 +111,17 @@ module controller #(
                             controller_state[i] <= WRITE_RELAYING;
                         end
                     end
-                    // Wait until consumer acknowledges it received data, then reset
+                    // Wait until consumer acknowledges it received response, then reset
                     READ_RELAYING: begin
                         if (!consumer_read_valid[current_consumer[i]]) begin 
-                            channel_serving_consumer[current_consumer[i]] <= 0;
+                            channel_serving_consumer[current_consumer[i]] = 0;
                             consumer_read_ready[current_consumer[i]] <= 0;
                             controller_state[i] <= IDLE;
                         end
                     end
                     WRITE_RELAYING: begin 
                         if (!consumer_write_valid[current_consumer[i]]) begin 
-                            channel_serving_consumer[current_consumer[i]] <= 0;
+                            channel_serving_consumer[current_consumer[i]] = 0;
                             consumer_write_ready[current_consumer[i]] <= 0;
                             controller_state[i] <= IDLE;
                         end

src/core.sv

@@ -14,36 +14,37 @@ module core #(
 ) (
     input wire clk,
     input wire reset,
+
+    // Kernel Execution
     input wire start,
     output wire done,
 
-    // METADATA
+    // Block Metadata
     input wire [7:0] block_id,
     input wire [$clog2(THREADS_PER_BLOCK):0] thread_count,
 
-    // PROGRAM MEMORY
+    // Program Memory
     output reg program_mem_read_valid,
     output reg [PROGRAM_MEM_ADDR_BITS-1:0] program_mem_read_address,
     input reg program_mem_read_ready,
     input reg [PROGRAM_MEM_DATA_BITS-1:0] program_mem_read_data,
 
-    // DATA MEMORY
+    // Data Memory
     output reg [THREADS_PER_BLOCK-1:0] data_mem_read_valid,
     output reg [DATA_MEM_ADDR_BITS-1:0] data_mem_read_address [THREADS_PER_BLOCK-1:0],
     input reg [THREADS_PER_BLOCK-1:0] data_mem_read_ready,
     input reg [DATA_MEM_DATA_BITS-1:0] data_mem_read_data [THREADS_PER_BLOCK-1:0],
-
     output reg [THREADS_PER_BLOCK-1:0] data_mem_write_valid,
     output reg [DATA_MEM_ADDR_BITS-1:0] data_mem_write_address [THREADS_PER_BLOCK-1:0],
     output reg [DATA_MEM_DATA_BITS-1:0] data_mem_write_data [THREADS_PER_BLOCK-1:0],
     input reg [THREADS_PER_BLOCK-1:0] data_mem_write_ready
 );
-    // STATE
+    // State
     reg [2:0] core_state;
     reg [2:0] fetcher_state;
     reg [15:0] instruction;
 
-    // EXECUTION
+    // Intermediate Signals
     reg [7:0] current_pc;
     wire [7:0] next_pc[THREADS_PER_BLOCK-1:0];
     reg [7:0] rs[THREADS_PER_BLOCK-1:0];
@@ -52,12 +53,14 @@ module core #(
     reg [7:0] lsu_out[THREADS_PER_BLOCK-1:0];
     wire [7:0] alu_out[THREADS_PER_BLOCK-1:0];
 
-    // DECODER
+    // Decoded Instruction Signals
     reg [3:0] decoded_rd_address;
     reg [3:0] decoded_rs_address;
     reg [3:0] decoded_rt_address;
     reg [2:0] decoded_nzp;
     reg [7:0] decoded_immediate;
+
+    // Decoded Control Signals
     reg decoded_reg_write_enable;           // Enable writing to a register
     reg decoded_mem_read_enable;            // Enable reading from memory
     reg decoded_mem_write_enable;           // Enable writing to memory
@@ -68,6 +71,7 @@ module core #(
     reg decoded_pc_mux;                     // Select source of next PC
     reg decoded_ret;
 
+    // Fetcher
     fetcher #(
         .PROGRAM_MEM_ADDR_BITS(PROGRAM_MEM_ADDR_BITS),
         .PROGRAM_MEM_DATA_BITS(PROGRAM_MEM_DATA_BITS)
@@ -84,6 +88,7 @@ module core #(
         .instruction(instruction) 
     );
 
+    // Decoder
     decoder decoder_instance (
         .clk(clk),
         .reset(reset),
@@ -105,6 +110,7 @@ module core #(
         .decoded_ret(decoded_ret)
     );
 
+    // Scheduler
     scheduler #(
         .THREADS_PER_BLOCK(THREADS_PER_BLOCK),
     ) scheduler_instance (
@@ -122,9 +128,11 @@ module core #(
         .done(done)
     );
 
+    // Dedicated ALU, LSU, registers, & PC unit for each thread this core has capacity for
     genvar i;
     generate
         for (i = 0; i < THREADS_PER_BLOCK; i = i + 1) begin : threads
+            // ALU
             alu alu_instance (
                 .clk(clk),
                 .reset(reset),
@@ -137,6 +145,7 @@ module core #(
                 .alu_out(alu_out[i])
             );
 
+            // LSU
             lsu lsu_instance (
                 .clk(clk),
                 .reset(reset),
@@ -158,6 +167,7 @@ module core #(
                 .lsu_out(lsu_out[i])
             );
 
+            // Register File
             registers #(
                 .THREADS_PER_BLOCK(THREADS_PER_BLOCK),
                 .THREAD_ID(i),
@@ -180,6 +190,7 @@ module core #(
                 .rt(rt[i])
             );
 
+            // Program Counter
             pc #(
                 .DATA_MEM_DATA_BITS(DATA_MEM_DATA_BITS),
                 .PROGRAM_MEM_ADDR_BITS(PROGRAM_MEM_ADDR_BITS)

src/dcr.sv

@@ -12,6 +12,7 @@ module dcr (
     input wire [7:0] device_control_data,
     output wire [7:0] thread_count,
 );
+    // Store device control data in dedicated register
     reg [7:0] device_conrol_register;
     assign thread_count = device_conrol_register[7:0];
 

src/decoder.sv

@@ -11,14 +11,14 @@ module decoder (
     input reg [2:0] core_state,
     input reg [15:0] instruction,
 
-    // Values
+    // Instruction Signals
     output reg [3:0] decoded_rd_address,
     output reg [3:0] decoded_rs_address,
     output reg [3:0] decoded_rt_address,
     output reg [2:0] decoded_nzp,
     output reg [7:0] decoded_immediate,
 
-    // Signals
+    // Control Signals
     output reg decoded_reg_write_enable,           // Enable writing to a register
     output reg decoded_mem_read_enable,            // Enable reading from memory
     output reg decoded_mem_write_enable,           // Enable writing to memory
@@ -28,7 +28,7 @@ module decoder (
     output reg decoded_alu_output_mux,             // Select operation in ALU
     output reg decoded_pc_mux,                     // Select source of next PC
 
-    // Done
+    // Return (finished executing thread)
     output reg decoded_ret
 );
     localparam NOP = 4'b0000,
@@ -62,12 +62,14 @@ module decoder (
         end else begin 
             // Decode when core_state = DECODE
             if (core_state == 3'b010) begin 
+                // Get instruction signals from instruction every time
                 decoded_rd_address <= instruction[11:8];
                 decoded_rs_address <= instruction[7:4];
                 decoded_rt_address <= instruction[3:0];
                 decoded_immediate <= instruction[7:0];
                 decoded_nzp <= instruction[11:9];
 
+                // Control signals reset on every decode and set conditionally by instruction
                 decoded_reg_write_enable <= 0;
                 decoded_mem_read_enable <= 0;
                 decoded_mem_write_enable <= 0;
@@ -78,6 +80,7 @@ module decoder (
                 decoded_pc_mux <= 0;
                 decoded_ret <= 0;
 
+                // Set the control signals for each instruction
                 case (instruction[15:12])
                     NOP: begin 
                         // no-op

src/dispatch.sv

@@ -13,26 +13,33 @@ module dispatch #(
     input wire reset,
     input wire start,
 
+    // Kernel Metadata
     input wire [7:0] thread_count,
+
+    // Core States
     input reg [NUM_CORES-1:0] core_done,
     output reg [NUM_CORES-1:0] core_start,
     output reg [NUM_CORES-1:0] core_reset,
     output reg [7:0] core_block_id [NUM_CORES-1:0],
     output reg [$clog2(THREADS_PER_BLOCK):0] core_thread_count [NUM_CORES-1:0],
+
+    // Kernel Execution
     output reg done
 );
+    // Calculate the total number of blocks based on total threads & threads per block
     wire [7:0] total_blocks;
     assign total_blocks = (thread_count + THREADS_PER_BLOCK - 1) / THREADS_PER_BLOCK;
 
-    reg [7:0] blocks_dispatched;
-    reg [7:0] blocks_done;
-    reg start_execution;
+    // Keep track of how many blocks have been processed
+    reg [7:0] blocks_dispatched; // How many blocks have been sent to cores?
+    reg [7:0] blocks_done; // How many blocks have finished processing?
+    reg start_execution; // EDA: Unimportant hack used because of EDA tooling
 
     always @(posedge clk) begin
         if (reset) begin
             done <= 0;
-            blocks_dispatched <= 0;
-            blocks_done <= 0;
+            blocks_dispatched = 0;
+            blocks_done = 0;
             start_execution <= 0;
 
             for (int i = 0; i < NUM_CORES; i++) begin
@@ -42,15 +49,15 @@ module dispatch #(
                 core_thread_count[i] <= THREADS_PER_BLOCK;
             end
         end else if (start) begin    
-            // Indirect way to get posedge start without driving from 2 cycles
-            // TODO: Remove this ugly code
+            // EDA: Indirect way to get @(posedge start) without driving from 2 different clocks
             if (!start_execution) begin 
                 start_execution <= 1;
                 for (int i = 0; i < NUM_CORES; i++) begin
                     core_reset[i] <= 1;
                 end
             end
 
+            // If the last block has finished processing, mark this kernel as done executing
             if (blocks_done == total_blocks) begin 
                 done <= 1;
             end
@@ -67,22 +74,17 @@ module dispatch #(
                             ? thread_count - (blocks_dispatched * THREADS_PER_BLOCK)
                             : THREADS_PER_BLOCK;
 
-                        blocks_dispatched <= blocks_dispatched + 1;
-
-                        // Only dispatch one block per clock cycle, so multiple cores can't pickup the same block
-                        break;
+                        blocks_dispatched = blocks_dispatched + 1;
                     end
                 end
             end
 
             for (int i = 0; i < NUM_CORES; i++) begin
                 if (core_start[i] && core_done[i]) begin
+                    // If a core just finished executing it's current block, reset it
                     core_reset[i] <= 1;
                     core_start[i] <= 0;
-                    blocks_done <= blocks_done + 1;
-
-                    // Only update one block per cycle
-                    break;
+                    blocks_done = blocks_done + 1;
                 end
             end
         end