1
\$\begingroup\$

I have attempted to implement a 16-bit RISC processor in Verilog, utilizing a two-dimensional register to implement a bank of 16 16-bit registers.

case(opcode)
 4'b0000: registers[res] <= registers[reg1] + registers[reg2]; // ADD
 4'b0001: registers[res] <= registers[reg1] - registers[reg2]; // SUB
 4'b0010: registers[res] <= registers[reg1] * registers[reg2]; // MUL
 4'b0011: begin
 if (registers[reg2] == 16'd0)
 error <= 1;
 else
 registers[res] <= registers[reg1] / registers[reg2]; // DIV
 end
 4'b0100: begin
 if (registers[reg2] == 16'd0)
 error <= 1;
 else
 registers[res] <= registers[reg1] % registers[reg2]; // MOD
 end
 4'b0101: begin // COMPARISON
 if (registers[reg1] > registers[reg2])
 GLE <= 3'b100;
 else if (registers[reg1] < registers[reg2])
 GLE <= 3'b010;
 else
 GLE <= 3'b001;
 end
 4'b0110: registers[res] <= registers[reg1] | registers[reg2]; // OR
 4'b0111: registers[res] <= registers[reg1] & registers[reg2]; // AND
 4'b1000: registers[res] <= registers[reg1] ^ registers[reg2]; // XOR
 4'b1001: registers[res] <= ~registers[reg1]; // NOT
 4'b1010: registers[res] <= registers[reg1]; // MOVE
 4'b1011: registers[res] <= registers[reg1] << registers[reg2]; // LEFT SHIFT
 4'b1100: registers[res] <= registers[reg1] >> registers[reg2]; // RIGHT SHIFT
 4'b1101: registers[res] <= memory[reg1]; // LOAD (register = memory)
 4'b1110: memory[res] <= registers[reg1]; // STORE (memory = register)
endcase

All operations are selected using the case structure as shown above. The registers are declared as shown below:

reg [15:0] registers[0:15];
reg [15:0] memory[0:15];
reg error = 0;

Here 'registers' is a bank of registers to perform operations with, 'memory' allows for memory storage and 'error' is an error flag.

The problem: none of the registers are updated when a valid opcode passed. For example, if opcode 0000 is passed, along with values for reg1, reg2 and res, nothing happens and 'registers' simply retains the values it was initialised with. Why is this happening?

The problem is specifically with the registers, i was able to confirm all opcodes and values for res, reg1 and reg2 are passed correctly by debugging with $display statements. The case structure lies inside an always block triggered by posedge clock.

Here is the full code for reference:

module RISC16(
 input clk,
 input [15:0] in,
 input [255:0] memin_flat,
 input [255:0] regin_flat,
 output reg [2:0] GLE,
 output reg [255:0] registers_flat,
 output reg [255:0] memory_flat,
 output reg error_flag
);
 reg [3:0] opcode = 4'b0;
 reg [3:0] reg1 = 4'b0;
 reg [3:0] reg2 = 4'b0;
 reg [3:0] res = 4'b0;
 reg [15:0] registers[0:15];
 reg [15:0] memory[0:15];
 reg error = 0;
 integer i;
 // Clocked process to update registers and memory
 always @(posedge clk) begin
 // Unflatten regin_flat into registers
 for (i = 0; i < 16; i = i + 1) begin
 registers[i] <= regin_flat[i*16 +: 16]; // Non-blocking assignment (<=)
 end
 // Unflatten memin_flat into memory
 for (i = 0; i < 16; i = i + 1) begin
 memory[i] <= memin_flat[i*16 +: 16]; // Non-blocking assignment (<=)
 end
 // Decode the input instruction
 opcode <= in[15:12]; // Non-blocking assignments
 reg1 <= in[11:8];
 reg2 <= in[7:4];
 res <= in[3:0];
 GLE <= 3'b000;
 error <= 0; // Reset error signal
 // Perform the operation based on opcode
 case(opcode)
 4'b0000: registers[res] <= registers[reg1] + registers[reg2]; // ADD
 4'b0001: registers[res] <= registers[reg1] - registers[reg2]; // SUB
 4'b0010: registers[res] <= registers[reg1] * registers[reg2]; // MUL
 4'b0011: begin
 if (registers[reg2] == 16'd0)
 error <= 1;
 else
 registers[res] <= registers[reg1] / registers[reg2]; // DIV
 end
 4'b0100: begin
 if (registers[reg2] == 16'd0)
 error <= 1; 
 else
 registers[res] <= registers[reg1] % registers[reg2]; // MOD
 end
 4'b0101: begin // COMPARISON
 if (registers[reg1] > registers[reg2])
 GLE <= 3'b100;
 else if (registers[reg1] < registers[reg2])
 GLE <= 3'b010;
 else
 GLE <= 3'b001;
 end
 4'b0110: registers[res] <= registers[reg1] | registers[reg2]; // OR
 4'b0111: registers[res] <= registers[reg1] & registers[reg2]; // AND
 4'b1000: registers[res] <= registers[reg1] ^ registers[reg2]; // XOR
 4'b1001: registers[res] <= ~registers[reg1]; // NOT
 4'b1010: registers[res] <= registers[reg1]; // MOVE
 4'b1011: registers[res] <= registers[reg1] << registers[reg2]; // LEFT SHIFT
 4'b1100: registers[res] <= registers[reg1] >> registers[reg2]; // RIGHT SHIFT
 4'b1101: registers[res] <= memory[reg1]; // LOAD (register = memory)
 4'b1110: memory[res] <= registers[reg1]; // STORE (memory = register)
 endcase
 $display("Opcode: %b, reg1: %b, reg2: %b, res: %b", opcode, registers[reg1], registers[reg2], registers[res]);
 // Flatten registers array into registers_flat
 for (i = 0; i < 16; i = i + 1) begin
 registers_flat[i*16 +: 16] <= registers[i]; // Non-blocking assignment
 end
 // Flatten memory array into memory_flat
 for (i = 0; i < 16; i = i + 1) begin
 memory_flat[i*16 +: 16] <= memory[i]; // Non-blocking assignment
 end
 // Set error flag
 error_flag <= error; 
 end
endmodule
toolic
10.8k11 gold badges31 silver badges35 bronze badges
asked Nov 23, 2024 at 19:15
\$\endgroup\$
0

1 Answer 1

3
\$\begingroup\$

The Verilog simulation does not behave as you expect because you are not following good Verilog coding practices. Specifically, you should not make multiple nonblocking assignments to the same signal. While it is legal, it should be avoided because it can easily lead to unexpected results such as yours.

This line of code makes assignments to the 16 registers signals:

 registers[i] <= regin_flat[i*16 +: 16]; // Non-blocking assignment (<=)

The following line makes an assignment to one of the same 16 signals (assuming opcode=0 and depending on the value of res):

 4'b0000: registers[res] <= registers[reg1] + registers[reg2]; // ADD

The same is true for the other case items assignments to registers.

You should re-work the code to avoid these multiple assignments. I recommend starting over, just trying to get one opcode to work with one 16-bit register (instead of a register file) for now.

Another general recommendation is to split the single always block into multiple always blocks. This usually makes the code easier to understand and debug.

I also recommend that you debug your code by dumping simulation waveforms to get a more complete picture of why the code does not work. It is usually insufficient to debug only by using $display.

answered Nov 23, 2024 at 22:28
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.