Verilog error : A reference to a wire or reg is not allowed in a constant expression

Question

I'm new to Verilog and I would really appreciate it if someone could help me out with this error:

output  reg [0:image_width][image_height:0]    result
....
integer i, j, imageX, imageY, x, y, kernelX, kernelY;
....

@(negedge ACLK)
for(x = 0; x < image_width; x++) begin 
    for(y = 0; y < image_height; y++)
    begin
    //multiply every value of the filter with corresponding image pixel
        for(kernelX = 0; kernelX < kernel_width; kernelX++) begin            
            for(kernelY = 0; kernelY < kernel_height; kernelY++)
                begin           
                    imageX = (x - kernel_width / 2 + kernelX + image_width) % image_width;
                    imageY = (y - kernel_height / 2 + kernelY + image_height) % image_height;

                    // ignore input samples which are out of bound
                    if( imageY >= 0 && imageY < image_height && imageX >= 0 && imageX < image_width )
                        //ERROR HERE!!!
                        result[x][y] += image[imageX][imageY] * kernel[kernelX][kernelY];   
                    end
                end
        end
    end
end

The error I get is:

error: A reference to a wire or reg ('x') is not allowed in a constant expression.
error: Array index expressions must be constant here.
error: A reference to a wire or reg ('imageX') is not allowed in a constant expression.
error: Array index expressions must be constant here.
error: A reference to a wire or reg ('kernelX') is not allowed in a constant expression.
error: Array index expressions must be constant here.

Could somebody tell me what I'm doing wrong? Thank you!

Are you sure `image_width` and `image_height` are parameters? — sharvil111, Jul 04 '16 at 05:42
Yes, they are. `module PU_conv ` `#( // Parameters` `parameter integer image_width = 10,` `parameter integer image_height = 4,` `parameter integer kernel_width = 2,` `parameter integer kernel_height = 2` `)` — AnnaR, Jul 04 '16 at 05:44

score 1 · Accepted Answer · edited May 23 '17 at 11:46

This line is the problem:

result[x][y] += image[imageX][imageY] * kernel[kernelX][kernelY];

Indexing into arrays is only allowed for constant expressions. You are not allowed to use variables in vector indexes. Remember that you're working with an HDL: you're dictating physical connections in hardware. Having a variable in the index implies the ability to dynamically rewire the circuit. This SO question has some rough workarounds that may work for you. However, you should really try to refactor your algorithm to avoid the need to use the variable indexing in the first place.

By the way, you should be using non-blocking assignments instead of the blocking assignments you currently have. Your code is in a clocked block, so blocking combinational logic should be avoided:

imageX <= (x - kernel_width / 2 + kernelX + image_width) % image_width;
imageY <= (y - kernel_height / 2 + kernelY + image_height) % image_height;

// ignore input samples which are out of bound
if( imageY >= 0 && imageY < image_height && imageX >= 0 && imageX < image_width )
    result[x][y] <= result[x][y] + image[imageX][imageY] * kernel[kernelX][kernelY];

Thank you so much! Would you mind telling me how I can fix this? I haven't been able to figure it out. — AnnaR, Jul 04 '16 at 17:07
@AnnaR I suggest you go visit someone who knows Verilog in person to get help with this. Your TA or professor should be able to help. You'll need to rethink your whole algorithm to eliminate the need to use variables to index into your vectors. If you can allow them to be constants somehow, then you can use a genvar to statically connect your function calls. I'm also suggesting this because your code needs extensive improvements - they can help you understand blocking vs nonblocking assignments, and having four nested loops suggests to me that there is a better way to do this. — skrrgwasme, Jul 04 '16 at 19:39

score 0 · Answer 2 · answered Jul 04 '16 at 06:10

0

@(negedge ACLK);
               ^

I'm pretty sure that semicolon doesn't belong there. As written, the for loops are all outside the always block.

Additionally, your image array currently only has one bit per pixel. Is this intentional? Whether it is or not, I would recommend that you reconsider this architecture; filtering an image of any significant size in a single clock cycle is not going to synthesize very well.

answered Jul 04 '16 at 06:10

I'm sorry, that was a typo. I've fixed it. I'm still getting the same error. All the for loops are inside the task. Also, no, I need more than 1 bit per pixel, is there something wrong with the way I've declared the array? – AnnaR Jul 04 '16 at 06:39
You're declaring the array as a `image_width` x `image_height` array of `reg`s. A register is one bit. – Jul 04 '16 at 18:55

Verilog error : A reference to a wire or reg is not allowed in a constant expression

2 Answers2