[SOLVED] Verilog Error : Too Few Parameters Passed To Task

[AshkanYJM]

The last link worked great, much better than the BRAM PDF I read in CoreGen.

Now I have chosen to use the simple dual port ram. two ports, 1 for in,1 for out, 2 clocks. exactly what I needed.

Now guys, do you see this approach appropriate according to what has been said till now ? it's not the exact code just the abstract.

reg [11:0] B;
reg [11:0] A;
reg [11:0] C;
input [11:0] Time_Delay;
integer k;

always@(posedge clka)//for the input port A
begin
if(addra==12'd4096)
addra=0;
else
addra=addra+1;
end
end
end

// for echo 

always@(posedge clkb)//the output port B
begin
addrb=0;
B<=doutb;
repeat(1)
addrb = Time_Delay;
end

//for chorus

always@(posedge clkb)
begin
addrb=0;
A<=doutb;
repeat(1)
for (k=1;k<=801;k=k+1)
case (k)
1: addrb=12'd1600;
2: addrb=12'd1604;
....
endcase
end
B<=addrb;
B=B>>1;
C=A+B;
end

 

[ads-ee]

it's not the exact code just the abstract.

It better be just an abstract, because it's written like a software program (again). You should be drawing a schematic or block diagram and timing diagrams not an abstract like this. As you have your "for loop" indexing with a start value of 1....(screams loudly..I AM A SOFTWARE PROGRAMMER). Hardware types always start with 0 as it's a valid value for addresses to memories, counter start values, the value after resetting any arbitrary register, etc. Think of it this way: 00-01-10-11 counts to 4 in 2-bits, 001-010-011-100 counts to 4 in 3-bits, which is easier to code and which is most efficient? (hint: the first one with 2-bits)

Don't use two (asynchronous) clocks, especially as you don't appear to be that familiar with digital design techniques. Use a single global clock that is at the highest frequency you need. Use clock enables on every other domain that doesn't need to run at the full clock rate. Use a simple dual port with a common clock.

Your address compare addra == 12'd4096 is actually comparing to 0. 4096 is 13'b1_0000_0000_0000 in binary, therefore 12'd4096=12'd0. You count between 0-4095 for 12-bits.

And once again I see a for loop (yes I know this is an abstract). Don't use a for loop. Let me repeat that, don't use a for loop, use an FSM or a counter or something else. k should probably be a counter like: k <= k+1; with whatever appropriate code around it to enable or clear the count. Even easier addrb <= addrb + 4; If the addrb is supposed to be a sine wave then use a ROM and get rid of the case statement or bury the ROM case statement into a submodule called something like sine_lut (this would be an appropriate case of moving code into a module to reduce 100's of lines of mind numbing code).

If you are thinking of this sequentially (like software) this won't work in Verilog:

B<=addrb;
B=B>>1;
C=A+B;

Along with assigning the addrb in two separate always blocks, that won't work either, this isn't like software, where you can assign values to globals any place in the code you want.

Regards

 

[mrflibble]

You really should RTFM sometimes. Make that yesterday.

https://www.asic-world.com/verilog/vbehave3.html

Don't use repeat statements. See the header on that linked page? "Verilog Behavioral Modeling". Behavioral. Modeling. No synthesizing for you my friend.

How about the following habit: every single flipping statement you use for the first time, and do not know for sure what it does ... google it, read the verilog LRM for a bit. Until you know what the hell it actually does. And then you use it. Because your current way of operation is actually slower than doing that. RTFM takes 15 minutes. Trial and error + forum reply round trip takes far longer than that. Plus by reading the fine manual you actually learn to solve problems by yourself.

Do you have a book or syllabus on digital ... well anything really. Because as ads-ee points out, anyone with their hardware description hat on would see a 12-bit counter that overflows (4095+1), and the counter wraps around to 0. So describe it as such, no need for if-then like waffle as you would do in C. Hell, even in somewhat optimized C you would use a bitmask.

So as said: k should probably be a counter like: k <= k+1;

Especially at your current stage, you really should get into the habit of using non-blocking statement.
1 - read this: https://www.asic-world.com/tidbits/blocking.html
2 - google this in general: verilog blocking non blocking

Why should you use non-blocking? It will help you avoid your current mind-set of sequential statements. And not only that, it also has a higher chance of producing non-shit code that is maintainable & readable as a bonus.

So taking the k counter as an example ... don't use anything like "k = k+1;". You should use "k <= k+1;" instead.

- - - Updated - - -

And taking your above code.

Don't do something like this:

addrb=0;
B<=doutb;
repeat(1)
addrb = Time_Delay;

But do something like this instead:

B <= doutb;
addrb <= Time_Delay;

 

[AshkanYJM]

Hey Guys, Long Time No See! It's been a while now, yeah I was too busy and as you can see(attached file) Things have changed, Revolution is here...

It is very consuming to explain this, as I am sure you will understand what I have done(you know me)
I just need to mention that clocks are not done right,not worrying about that right now. The problem here is to put raw data in my chorusrom and phaserrom by $readmemb but when synthesis and implementation check ISE says this:

[COLOR="#FF0000"]ERROR:HDLCompilers:28 - "Multieffect.v" line 92 'CHORUSROM' has not been declared
ERROR:HDLCompilers:28 - "Multieffect.v" line 93 'PHASERROM' has not been declared[/COLOR]

I am sure that i have put my text files (e.g chorusrom.txt) in the correct directory and that they are aligned correctly too !

Just can't figure it out where to put this $readmemb command, it does not even fit into the ROMS .v files.
I've searched a lot and i reckon this old chum $readmemb is just right to use when you define roms like [12:0] Mem[255:0] ! not with Coregenic ROMS!

Regards

 

[FvM]

CHORUSROM u1(.clk(Clk2x),.rsta(Reset),.addra(g1),.douta(w2));
PHASERROM u2(.clk(Clk2x),.rsta(Reset),.addra(g2),.douta(w3));

How do the queried source lines look like? In my view they are module instantiations. So modules with entity names CHORUSROM and PHASERROM must exist in your design space, either verilog code, coregen generated or whatsoever.

A readmemb() statement does no define a memory module, it can only initialize an exiting memory object. My personal favourite in this case would be inferred memory, an array object in Verilog code with synchronous read in an always block, using readmemb() for initiaization. I'm sure you'll find an example.

I notice that this pretty long thread is going on since more than two weeks and has become something like a private blog "My exciting adventures in the land of Verilog", but not tracking a clear topic. It's unlikely to return to the original topic, only arbitrary new questions can be expected to follow. I think we should better close the thread.

 

[ads-ee]

You aren't pointing to the name of the memory array.

CHORUSROM u1(.clk(Clk2x),.rsta(Reset),.addra(g1),.douta(w2));
$readmemb("chorus.txt",CHORUSROM,0,796);

CHORUSROM is the module name of the u1 isntance. It is not the name of a [12:0] Mem[255:0] array.

Take a look at the following information on using readmenb: **broken link removed** as you can see the array memory is what is used.
What is wrong with using a core generator ROM? You seem against using it.

Other issues that would preclude having only one error.
1) You are assigning input ports inside the module, which is illegal in Verilog.

input [10:0] addra1;
input [10:0] addra2;
input [11:0] addra3;
always @(posedge clkdv) begin
  if (addra1==800)
    addra1=0;
  else
    addra1=addra1+1;
end

You are also using blocking assignments when you should be using non-blocking assignments (you still need to read and understand the contents of a verilog book) as this is an edge triggered sequential block.

2) You are performing and assignment using assign to a reg type, This is illegal in Verilog.

reg [10:0]g1,g2;
reg [11:0]g3;
assign g1=addra1;
assign g2=addra2;
assign g3=addra3;

I don't even understand why you would even perform this assignment as it does nothing but change the name of the signal. IMO this is a waste of typing.

3) Still using the mux4to2, which is both poorly named multiplexers have 1 output not 2 and used low level Verilog gates which are defined for scalar values. As you allow for behavioral code at the top level the mux should just be more behavioral code (i.e. a case statement) at the top level.

4) You need to learn how to write self documenting code...tip1 use names that have meaning i.e. names like w6 don't tell someone what the signal does like the name enable, which is probably an enable. Lack of comments is a problem too, makes code a maintenance nightmare.

5) Way too many intermediate signal names e.g.

assign t3=t2 >> 1;
assign w8=t3;
// this should have been one line of code:
assign w8 = t2 >> 1;

and what is this? Wrong! No!, learn Verilog read a book!

always @(posedge Clk) begin
  t1<=w6;
  t2<=w6;
  [B][I]// What is an assign doing in an edge triggered sequential block, this is plain wrong.[/I]
  assign t3=t2 >> 1;[/B]
end

6) You seem to be taking selective forum advice and ignoring the reset, including a lot of the advice I've given you. Why should I continue bothering replying to these posts, if I'm wasting my time? Unless I see everything in this post being addressed by you I will give up replying as my time is more valuable spent elsewhere.

Regards

 

[mrflibble]

@AshkanYJM:
Maybe you have an ads-ee filter in your brain or something, because being corrected so often can cause some people to become pouty. But that's just too bad because he happens to be right.

You do seem to ignore a lot of advice. Just the assign in the last quote. WTF? Didn't we point out that 1) it's a bad idea in your current stage and 2) didn't I suggest some reading material as to why? And not only that, that assign is just plain wrong as in invalid verilog code. Read. A. Book.

You really should RTFM sometimes. Make that yesterday.

...

Especially at your current stage, you really should get into the habit of using non-blocking statement.
1 - read this: https://www.asic-world.com/tidbits/blocking.html
2 - google this in general: verilog blocking non blocking

Why should you use non-blocking? It will help you avoid your current mind-set of sequential statements. And not only that, it also has a higher chance of producing non-shit code that is maintainable & readable as a bonus.

I'm not inclined to give a huge explanation as to why. I just point out that something is a bad idea for you right now, and then you can read about why. If you don't then that's just too bad. It's your verilog that is going to generate vast amounts of vacuum, not mine.

Potential reply: "But but I cannot afford the time it takes to read books, I am in a hurry! My deadline is yesterday!"
Counter reply: "Yes yes, it's not as if that false argument is anything new. By not taking the time to read a book you can allocate more time to failing. While simultaneously being guaranteed to take longer before you arrive at a decent working solution. Steady work wins the race, not crappy uninformed hurry up work."

Sooo, I'd suggest you re-read the given advice in this thread, and actually implement said advice.

Good luck!

 

[AshkanYJM]

You are right, The Codes Had Mistakes But I hadn't claimed otherwise, I just asked about those $readmembs. anyway I feel obliged to thank you both since you take time to check my poorly-written codes and type the errors and advice here. I really appreciate that.

Well somebody come and make this thread SOLVED. since that error,,,, Too Few Parameters Passed To Task,,, was solved way too many years ago. I am about to finish the project,just ABOUT to! If I had any other problems I would ask it in another thread,Just After I'm helpless enough searching everywhere making sure my answer is not there !


Regards