can anyone tell me what is wrong with this Verilog code ????

[angjohn]

parameter s7 = 5’b00111

following code is written using Verilog:

//CU
module Controlunit (Z, reset, CMP, OPCODE, CLK);

output[14:0] Z;
wire[14:0] Z;
input reset;
input CMP;
input[7:0] OPCODE;
input CLK;


//internal FSM state declarations, PS= Present state, NS=next state
reg[4:0] PS;
wire[4:0] NS;


//state encodings
parameter S0 = 5'b00000;
parameter S1 = 5'b00001;
parameter S2 = 5'b00010;
parameter S3 = 5'b00011;
parameter S4 = 5'b00100;
parameter S5 = 5'b00101;
parameter S6 = 5'b00110;
parameter S7 = 5'b00111;
parameter S8 = 5'b01000;
parameter S9 = 5'b01001;
parameter S10 = 5'b01010;
parameter S11 = 5'b01011;
parameter S12 = 5'b01100;
parameter S13 = 5'b01101;
parameter S14 = 5'b01110;
parameter S15 = 5'b01111;
parameter S16 = 5'b10000;
parameter S17 = 5'b10001;
parameter S18 = 5'b10010;
parameter S19 = 5'b10011;

wire[1:0] a;
wire[1:0] b;
wire c;
wire d;
wire[3:0] e;

assign a = OPCODE[7:6] ;
assign b = OPCODE[5:4] ;
assign c = OPCODE[3] ;
assign d = OPCODE[2] ;
assign e = OPCODE[7:4] ;


//combinational logic

function[19:0] fsm;
input[1:0] fsm_a;
input[1:0] fsm_b;
input fsm_c;
input fsm_d;
input[3:0] fsm_e;
input[4:0] fsm_PS;
input fsm_CMP;

reg[14:0] fsm_Z;
reg[4:0] fsm_NS;

begin
case (fsm_PS)
S0 :
begin
fsm_NS = S1 ;
fsm_Z = 15'b010000000000000 ;
end
S1 :
begin
fsm_NS = S2 ;
fsm_Z = 15'b000000000000000 ;
end
S2 :
begin
fsm_Z = 15'b101000000000000 ;
if (fsm_a == 2'b00 | fsm_a == 2'b01)
begin
fsm_NS = S3 ;
end
else if (fsm_a == 2'b10)
begin
fsm_NS = S10 ;
end
else if (fsm_a == 2'b11)
begin
fsm_NS = S19 ;
end
end
S3 :
begin
fsm_Z = 15'b000000000000000 ;
case (fsm_e)
4'b0000 :
begin
fsm_NS = S4 ;
end
4'b0001 :
begin
fsm_NS = S5 ;
end
4'b0010 :
begin
fsm_NS = S6 ;
end
4'b0011 :
begin
fsm_NS = S7 ;
end
4'b0100 :
begin
fsm_NS = S8 ;
end
default :
begin
fsm_NS = S9 ;
end
endcase
end
S4 :
begin
fsm_NS = S1 ;
if (fsm_c == 1'b0)
begin
fsm_Z = 15'b000000010001000 ;
end
else
begin
fsm_Z = 15'b000000001001000 ;
end
end
S5 :
begin
fsm_NS = S1 ;
if (fsm_c == 1'b0)
begin
fsm_Z = 15'b000000010001001 ;
end
else
begin
fsm_Z = 15'b000000001100001 ;
end
end
S6 :
begin
fsm_NS = S1 ;
if (fsm_c == 1'b0)
begin
fsm_Z = 15'b000000010001010 ;
end
else
begin
fsm_Z = 15'b000000001001010 ;
end
end
S7 :
begin
fsm_NS = S1 ;
if (fsm_c == 1'b0)
begin
fsm_Z = 15'b000000010001011 ;
end
else
begin
fsm_Z = 15'b000000001001011 ;
end
end
S8 :
begin
fsm_NS = S1 ;
if (fsm_c == 1'b0)
begin
fsm_Z = 15'b000000010000100 ;
end
else
begin
fsm_Z = 15'b000000001100100 ;
end
end
S9 :
begin
fsm_NS = S1 ;
if (fsm_c == 1'b0)
begin
fsm_Z = 15'b000000010000101 ;
end
else
begin
fsm_Z = 15'b000000001100101 ;
end
end
S10 :
begin
fsm_Z = 15'b000000000000110 ;
fsm_NS = S11 ;
end
S11 :
begin
fsm_Z = 15'b001000100000110 ;
case (fsm_b)
2'b00 :
begin
fsm_NS = S12 ;
end
2'b01 :
begin
fsm_NS = S15 ;
end
2'b10 :
begin
fsm_NS = S17 ;
end
default :
begin
fsm_NS = S18 ;
end
endcase
end
S12 :
begin
if (fsm_d == 1'b0)
begin
fsm_NS = S14 ;
fsm_Z = 15'b000000000000110 ;
end
else
begin
fsm_Z = 15'b000010000000110 ;
fsm_NS = S13 ;
end
end
S13 :
begin
fsm_NS = S14 ;
fsm_Z = 15'b000010100000110 ;
end
S14 :
begin
fsm_NS = S1 ;
if (fsm_c == 1'b0)
begin
fsm_Z = 15'b000000010010110 ;
end
else
begin
fsm_Z = 15'b000000001010110 ;
end
end
S15 :
begin
fsm_NS = S16 ;
if (fsm_c == 1'b0)
begin
fsm_Z = 15'b000011000000110 ;
end
else
begin
fsm_Z = 15'b000011000100110 ;
end
end
S16 :
begin
fsm_NS = S1 ;
fsm_Z = 15'b000010000000110 ;
end
S17 :
begin
fsm_NS = S1 ;
fsm_Z = 15'b000100000010110 ;
end
S18 :
begin
fsm_NS = S1 ;
if (fsm_CMP == 1'b1)
begin
fsm_Z = 15'b000100000010110 ;
end
else
begin
fsm_Z = 15'b000000000000000 ;
end
end
S19 :
begin
fsm_NS = S1 ;
fsm_Z = 15'b000000000000000 ;
end
endcase

fsm = {fsm_Z, fsm_NS};
end
endfunction


//reevaluate combinational logic whenever opcode or Present state changes
assign {Z, NS}=fsm(a,b,c,d,e,PS,CMP);


//clock the state flip flop
always @(negedge CLK)
begin
if (reset == 1'b1)
PS = S0 ;
else
PS = NS ;
end

endmodule



thanks for helping !!!!

 

[Thomson]

Re: can anyone tell me what is wrong with this Verilog code

(1) the case statements should be complete which means you should include all the possible possibilities ; or you can contain a Default assignments instead during which NS = PS;other variables' assignemnts depend the real conditions.

(2) do not use assign statements arbitarily since for VCS and Verilog-XL these statements' execution mechanism differ from always procedures; therefore please use always procedures as possible. This mainly affects your simulations results if using different simulators.

 

[echo47]

When you post code to this forum, please use the "code" button so we can see your indenting.

 

[nand_gates]

Re: can anyone tell me what is wrong with this Verilog code

Here is the corrected code!
This is what is ment by Mr. Thomson..
I made it little readable by removing unwanted begin ends

//CU
module Controlunit (Z, reset, CMP, OPCODE, CLK);

   output[14:0] Z;
   reg [14:0]  Z;
   input       reset;
   input       CMP;
   input [7:0] OPCODE;
   input       CLK;


   //internal FSM state declarations, PS= Present state, NS=next state
   reg [4:0]  PS;
   reg [4:0]  NS;


   //state encodings
   parameter   S0 = 5'b00000;
   parameter   S1 = 5'b00001;
   parameter   S2 = 5'b00010;
   parameter   S3 = 5'b00011;
   parameter   S4 = 5'b00100;
   parameter   S5 = 5'b00101;
   parameter   S6 = 5'b00110;
   parameter   S7 = 5'b00111;
   parameter   S8 = 5'b01000;
   parameter   S9 = 5'b01001;
   parameter   S10 = 5'b01010;
   parameter   S11 = 5'b01011;
   parameter   S12 = 5'b01100;
   parameter   S13 = 5'b01101;
   parameter   S14 = 5'b01110;
   parameter   S15 = 5'b01111;
   parameter   S16 = 5'b10000;
   parameter   S17 = 5'b10001;
   parameter   S18 = 5'b10010;
   parameter   S19 = 5'b10011;

   wire [1:0]  a;
   wire [1:0]  b;
   wire        c;
   wire        d;
   wire [3:0]  e;

   assign      a = OPCODE[7:6] ;
   assign      b = OPCODE[5:4] ;
   assign      c = OPCODE[3] ;
   assign      d = OPCODE[2] ;
   assign      e = OPCODE[7:4] ;


   //combinational logic
   //reevaluate combinational logic whenever opcode or Present state changes
   always @(CMP or PS or a or b or c or d or e)
     begin
         NS = PS;
         Z = 15'b000000000000000 ;
         case (PS)
             S0 :
               begin
                   NS = S1 ;
                   Z = 15'b010000000000000 ;
               end
             S1 :
               begin
                    NS = S2 ;
                    Z = 15'b000000000000000 ;
                end
             S2 :
               begin
                   Z = 15'b101000000000000 ;
                   if (a == 2'b00 | a == 2'b01)
                     NS = S3 ;
                   else if (a == 2'b10)
                     NS = S10 ;
                   else if (a == 2'b11)
                     NS = S19 ;
               end
             S3 :
               begin
                   Z = 15'b000000000000000 ;
                   case (e)
                       4'b0000 : NS = S4 ;
                       4'b0001 : NS = S5 ;
                       4'b0010 : NS = S6 ;
                       4'b0011 : NS = S7 ;
                       4'b0100 : NS = S8 ;
                       default : NS = S9 ;
                   endcase
               end
             S4 :
               begin
                   NS = S1 ;
                   if (c == 1'b0)
                     Z = 15'b000000010001000 ;
                   else
                     Z = 15'b000000001001000 ;
               end
             S5 :
               begin
                   NS = S1 ;
                   if (c == 1'b0)
                     Z = 15'b000000010001001 ;
                   else
                     Z = 15'b000000001100001 ;
               end
             S6 :
               begin
                   NS = S1 ;
                   if (c == 1'b0)
                     Z = 15'b000000010001010 ;
                   else
                     Z = 15'b000000001001010 ;
               end
             S7 :
               begin
                   NS = S1 ;
                   if (c == 1'b0)
                     Z = 15'b000000010001011 ;
                   else
                     Z = 15'b000000001001011 ;
               end
             S8 :
               begin
                   NS = S1 ;
                   if (c == 1'b0)
                     Z = 15'b000000010000100 ;
                   else
                     Z = 15'b000000001100100 ;
               end
             S9 :
               begin
                   NS = S1 ;
                   if (c == 1'b0)
                     Z = 15'b000000010000101 ;
                   else
                     Z = 15'b000000001100101 ;
               end
             S10 :
               begin
                   Z = 15'b000000000000110 ;
                   NS = S11 ;
               end
             S11 :
               begin
                   Z = 15'b001000100000110 ;
                   case (b)
                       2'b00 : NS = S12 ;
                       2'b01 : NS = S15 ;
                       2'b10 : NS = S17 ;
                       default : NS = S18 ;
                   endcase
               end
              S12 :
                begin
                    if (d == 1'b0)
                      begin
                          NS = S14 ;
                          Z = 15'b000000000000110 ;
                      end
                    else
                      begin
                          Z = 15'b000010000000110 ;
                          NS = S13 ;
                      end
                end
              S13 :
                begin
                    NS = S14 ;
                    Z = 15'b000010100000110 ;
                end
             S14 :
               begin
                   NS = S1 ;
                   if (c == 1'b0)
                     Z = 15'b000000010010110 ;
                   else
                     Z = 15'b000000001010110 ;
               end
             S15 :
               begin
                   NS = S16 ;
                   if (c == 1'b0)
                     Z = 15'b000011000000110 ;
                   else
                     Z = 15'b000011000100110 ;
               end
             S16 :
               begin
                   NS = S1 ;
                   Z = 15'b000010000000110 ;
               end
             S17 :
                begin
                    NS = S1 ;
                    Z = 15'b000100000010110 ;
                end
             S18 :
               begin
                   NS = S1 ;
                   if (CMP == 1'b1)
                     Z = 15'b000100000010110 ;
                   else
                     Z = 15'b000000000000000 ;
               end
             S19 :
               begin
                   NS = S1 ;
                   Z = 15'b000000000000000 ;
               end
         endcase
     end // always @ (...

   //clock the state flip flop
   always @(negedge CLK)
     begin
         if (reset == 1'b1)
           PS <= S0 ;
         else
           PS <= NS ;
     end

endmodule

 

[Thomson]

Re: can anyone tell me what is wrong with this Verilog code

(3)another problem maybe exist if you don't take care to write your RTL codes, which is that the blocking assignments are a better alternative to the non-blocking ones since the previous one willn't induce races (meaning these assignment does'nt dependent on your procedure's order as to simulation). for example the following two ones will cause races:
always @(posedge clock)
begin
a = b;
end
always @(posedge clock)
begin
c = a;
end

the results may be different for different vendors' simulators ; even for the same simulator, if the version differs the results may still be different. Because the execution order of the above two always blocks are at random, of cause if you use VCS simulator the results equal the following one:
always @(posedge clock)
begin
a = b;
c = a;
end


so if possible, using blocking assignments can reduce such write-read races and the order of the blocks can be ignored.


You can refer some booking disscussing such issues about how to avoid the races during your writting process.

Added after 21 minutes:

(4) Pay attention to the default assignments: NS = PS instead of NS = S0 please.
Given the condition that although some stimuli occurs but such stimuli doesn't alter the current state; therefore the NS = PS is the correct one.

 

[angjohn]

Re: can anyone tell me what is wrong with this Verilog code

thanks for all ur reply and it help, thanks!!! but now i have 1 more problem that is i had written this Program counter in VHDL then i want that module to be in Verilog but i had some problem writting in Verilog, following is the VHDL code and also the Verilog code that i had write but face some problem with it. so can anyone give me some advice on how to make the Verilog code similiar to the VHDL code, PLEASE !!!

VHDL:
-- PrgmCounter
library IEEE;
use IEEE.std_logic_1164.all;
use IEEE.std_logic_arith.all;
use IEEE.std_logic_unsigned.all;

entity PrgmCounter is
	port (
		data : in STD_LOGIC_VECTOR(7 downto 0);
		q : out STD_LOGIC_VECTOR(7 downto 0);
		clrreg : in STD_LOGIC;
		loadreg : in STD_LOGIC;
		increg : in STD_LOGIC;
		CLK : in STD_LOGIC
	);
end PrgmCounter;

architecture PrgmCounter_arch of PrgmCounter is

	signal temp: STD_LOGIC_VECTOR(7 downto 0);

begin
	process(clk, clrreg, loadreg, increg)
	begin
		if clrreg='1' then temp <= (others => '0');
		else
			if(clk' event and clk='1') then
				if loadreg='1' then temp <= data;
				else
					if increg='1' then temp <= temp+1;
					else temp <= temp;
					end if;
				end if;
			end if;
		end if;
	end process;
	q<=temp;
	
end PrgmCounter_arch;


Verilog:
// PrgmCounter
module PrgmCounter (data, q, clrreg, loadreg, increg, CLK);

   input[7:0] data; 
   output[7:0] q; 
   wire[7:0] q;
   input clrreg; 
   input loadreg; 
   input increg; 
   input CLK; 

   reg[7:0] temp; 

   always @(CLK or clrreg or loadreg or increg)
   begin
      if (clrreg == 1'b1)
      begin
         temp <= {8{1'b0}} ; 
      end
      else
      begin
         if (CLK == 1'b1)
         begin
            if (loadreg == 1'b1)
            begin
               temp <= data ; 
            end
            else
            begin
               if (increg == 1'b1)
               begin
                  temp <= temp + 1 ; 
               end
               else
               begin
                  temp <= temp ; 
               end 
            end 
         end 
      end 
   end 
   assign q = temp ;
endmodule

 

[jelydonut]

Re: can anyone tell me what is wrong with this Verilog code

3rd time trying to answer this.. stupid mandrake..

anyways..

the way you wrote your always block it would basically synthesize to combinatorial logic maybe with a inferred latch or two. I didn't pay attention.. What you are going for is a edge triggered clock event..

reg [7:0] whatever;

;always @(posedge clk) ; sync reset condition
always @(posedge clk or posedge rst_cond) ; async reset condition..
if(rst_cond)
whatever <= 8'b0000_0000;
else
begin
if(ld_cond)
whatever <= data_in;
else
if(inc_cond)
whatever <= whatever + 1'b1;
end

that should do it..

you also really don't need to go crazy on the begin-ends.. the only time it is required is if you have 2 statements inbetween branch conditions or if you have 2 branch conditions under a else..

example..

else
begin
if(inc_cond)
whatever <= whatever + 1'b1;
else
if(dec_cond)
whatever <= whatever - 1'b1;
end


jelydonut

 

[Thomson]

Re: can anyone tell me what is wrong with this Verilog code

There are some advices concerning the vering/VHDL coding styles:
(1) Think of the Hardware while coding the design
(2) Some discrepancies really exist among the Verilog and VHDL Syntax. The former always use posedge/negedge clk to indicate a clock event, while the latter ofter use the attribute of the signal to indicate a clock event such as clock'event. If you're a newer to the two languages, you'd better code your design when you'd understand the meaning of the effect of the sigans' events on the execution of the process(VHDL) or always(Verilog).
(3) There will not infer latches if you use non-blocking assignments. The latches infer often occur during the combinationatory logic generation blocks such as assign and always blocks, during which if the sensitivity list is not complete or the conditions are not complete, then the latches will be infered and the timing analysis will complicate and the difference between the RTL simulation and netlist simulation may (not must) occur.

 

[angjohn]

Re: can anyone tell me what is wrong with this Verilog code

thanks guys for the help !!!! i now face another problem, i am using this Altera Ram (LPM_RAM_DQ) and the following is the code i written and it works but due to the syntax:"defparam" i am using in the code some of the compiler cannot recognise that syntax. So, can anyone suggest to me how to replace that "defparam" syntax with a more generic syntax so i can use the code with most of the compiler, thanks!!! following is the code

module RAM (
	address,
	we,
	clock,
	data,
	q);

	input	[7:0]  address;
	input	we;
	input	clock;
	input	[7:0]  data;
	output	[7:0]  q;

	wire [7:0] sub_wire0;
	wire [7:0] q = sub_wire0[7:0];

	lpm_ram_dq	lpm_ram_dq_component (
				.address (address),
				.inclock (clock),
				.data (data),
				.we (we),
				.q (sub_wire0));
	defparam
		lpm_ram_dq_component.intended_device_family = "FLEX10K",
		lpm_ram_dq_component.lpm_width = 8,
		lpm_ram_dq_component.lpm_widthad = 8,
		lpm_ram_dq_component.lpm_indata = "REGISTERED",
		lpm_ram_dq_component.lpm_address_control = "REGISTERED",
		lpm_ram_dq_component.lpm_outdata = "UNREGISTERED",
		lpm_ram_dq_component.use_eab = "ON",
		lpm_ram_dq_component.lpm_hint = "MAXIMUM_DEPTH=256",
		lpm_ram_dq_component.lpm_type = "LPM_RAM_DQ",
		lpm_ram_dq_component.lpm_file = "nabilcpu.mif";


endmodule

or if anyone is familiar with the altera LPM_RAM_DQ can provide me the source code for it with more generic function!! thanks !!!

 

[angjohn]

Re: can anyone tell me what is wrong with this Verilog code

hey guys! i had write the following module in VHDL and it works well

-- Flag
library IEEE;
use IEEE.std_logic_1164.all;
use IEEE.std_logic_arith.all;

entity Flag is
	port (
		FlagIn : in STD_LOGIC_VECTOR(16 downto 0);
		ldOut : in STD_LOGIC;
		--ldStatus : in STD_LOGIC;
		FlagOut : out STD_LOGIC_VECTOR(15 downto 0);
		CLK : in STD_LOGIC
	);
end Flag;

architecture Flag_arch of Flag is
begin
	Process(clk) 
	variable C,Z,N,V : STD_LOGIC; 
	begin
	
		if(clk' event and clk ='1') then 
			if ldOut ='1' then
				if FlagIn = "00000000000000000" then
				   Z:='1';
				else
				   Z:='0';
				end if;
				
				--Bank := FlagIn(11);			
				N := FlagIn(15);
				C := FlagIn(16);
				V := N xor C;
				
				FlagOut <= C&N&V&Z&"000000000000";
			end if;
			
			
			--if ldStatus ='1' then
				--FlagOut <= FlagIn(15 downto 0);
			--end if;
			
		end if;
		
	end process;
end Flag_arch;

then i wan it to be in Verilog and i had use tranlator to convert it but it cannot work as i plan as above so can anyone help me modified the following Verilog code so that it work as exactly as the VHDL code above !!! thanks !!!!

// Flag
module Flag (FlagIn, ldOut, FlagOut, CLK);

   input[16:0] FlagIn; 
   input ldOut; 
   output[15:0] FlagOut; 
   reg[15:0] FlagOut;
   input CLK; 

   always @(posedge CLK)
   begin : 
      reg C; 
      reg Z; 
      reg N; 
      reg V; 

         if (ldOut == 1'b1)
         begin
            if (FlagIn == 17'b00000000000000000)
               Z = 1'b1; 
            else
               Z = 1'b0; 


            N = FlagIn[15]; 
            C = FlagIn[16]; 
            V = N ^ C; 
            FlagOut <= {C, N, V, Z, 12'b000000000000} ; 
         end 
   end 
endmodule

 

[nand_gates]

Re: can anyone tell me what is wrong with this Verilog code

Hi I got ur problem...
The translated code is giving compilation errors....
Here is the corrected code....

// Flag
module Flag (FlagIn, ldOut, FlagOut, CLK);

   input[16:0] FlagIn;
   input ldOut;
   output[15:0] FlagOut;
   reg[15:0] FlagOut;
   input CLK;

   always @(posedge CLK)
   begin : junk
      reg C;
      reg Z;
      reg N;
      reg V;

         if (ldOut == 1'b1)
         begin
            if (FlagIn == 17'b00000000000000000)
               Z = 1'b1;
            else
               Z = 1'b0;


            N = FlagIn[15];
            C = FlagIn[16];
            V = N ^ C;
            FlagOut <= {C, N, V, Z, 12'b000000000000} ;
         end
   end
endmodule

 

[tigerliu]

Re: can anyone tell me what is wrong with this Verilog code

Hi,
nand_gates' first part of code is good. This is an prefered coding style of statemachine. For more coding guideline ,please refer to:
h**p://www.sunburst-design.com/papers/



tiger