VHDL Code to Verilog Code

[knight]

I have written the Verilog version of the below CORDIC VHDL code, however I'm always getting 0 output for the sine wave. Can anyone please tell where is the error/logic error in my code.
Thanks

Edit: I have updated the code after help from hamster_nz and it's working fine now. Thanks everyone.
        The main difference was using >>> instead of >> (signed shift vs unsigned shift)

VHDL CODE

Code: [Select]

library IEEE;
use IEEE.STD_LOGIC_1164.ALL;
use IEEE.NUMERIC_STD.ALL;

entity sin_cos is
    Port ( clk     : in  STD_LOGIC;
           de_in   : in  std_logic; //not important, should be always 1                   
           angle   : in  STD_LOGIC_VECTOR (19 downto 0);
           de_out  : out std_logic := '0';  //again, not important
           sine    : out STD_LOGIC_VECTOR (19 downto 0)  := (others=>'0');
           cosine  : out STD_LOGIC_VECTOR (19 downto 0)  := (others=>'0'));
end sin_cos;

architecture Behavioral of sin_cos is
    type a_int   is array(0 to 15) of integer;
    constant angles : a_int := ( 77376, 40884, 20753, 10417,
                                  5213,  2607,  1304,   652,
                                   326,   163,    81,    41,
                                    20,    10,     5,     3);
    type a_value is array(0 to angles'high+1) of signed(19 downto 0);
    signal de_sr : std_logic_vector(angles'high+1 downto 0) := (others => '0');  //not important
    signal remainder : a_value   := (others => (others => '0'));
    signal x         : a_value  := (others => (others => '0'));
    signal y         : a_value := (others => (others => '0'));
begin

    de_out <= de_sr(de_sr'high);
    sine   <= std_logic_vector(x(x'high));
    cosine <= std_logic_vector(y(y'high));

process(clk)
    begin
        if rising_edge(clk) then
            for i in 0 to angles'high loop
               de_sr(i+1) <= de_sr(i);
               if remainder(i) < 0 then
                   remainder(i+1) <= remainder(i) + angles(i);
                   x(i+1)   <= x(i) + y(i)(19 downto i+1);
                   y(i+1)   <= y(i) - x(i)(19 downto i+1);
               else
                   remainder(i+1) <= remainder(i) - angles(i);
                   x(i+1)   <= x(i) - y(i)(19 downto i+1);
                   y(i+1)   <= y(i) + x(i)(19 downto i+1);
               end if;
            end loop;
           
            de_sr(0) <= de_in;
            if de_in = '1' then
                case angle(angle'high downto angle'high-1) is
                    when "00"   => x(0) <=  to_signed( 300000,20);  y(0) <=  to_signed( 300000,20); remainder(0) <= signed(angle) - x"20000";
                    when "01"   => x(0) <=  to_signed(-300000,20);  y(0) <=  to_signed( 300000,20); remainder(0) <= signed(angle) - x"60000";
                    when "10"   => x(0) <=  to_signed(-300000,20);  y(0) <=  to_signed(-300000,20); remainder(0) <= signed(angle) - x"A0000";
                    when others => x(0) <=  to_signed( 300000,20);  y(0) <=  to_signed(-300000,20); remainder(0) <= signed(angle) - x"E0000";
                end case;
            end if;
           
        end if;
    end process;
end Behavioral;

VERILOG CODE

Code: [Select]

module cordic(clk, angle, sine, cosine);
parameter N = 20;
input clk;
input signed [N-1:0] angle;
output [N-1:0] sine, cosine;

wire [N-1:0] angles [15:0];

assign angles[0] = 77376;
assign angles[1] = 40884;
assign angles[2] = 20753;
assign angles[3] = 10417;
assign angles[4] = 5213;
assign angles[5] = 2607;
assign angles[6] = 1304;
assign angles[7] = 652;
assign angles[8] = 326;
assign angles[9] = 163;
assign angles[10] = 81;
assign angles[11] = 41;
assign angles[12] = 20;
assign angles[13] = 10;
assign angles[14] = 5;
assign angles[15] = 3;

reg signed [N-1:0] remainder [15:0] = {default:0};
reg signed [N-1:0] x [15:0] = {default:0};
reg signed [N-1:0] y [15:0] = {default:0};
assign sine = x[15];
assign cosine = y[15];

integer i;
    always @(posedge clk)
    begin
    for (i=0;i<15;i=i+1)
    begin
    if (remainder[i] < 0)
    begin
        remainder[i+1] <= remainder[i] + angles[i];
        x[i+1]   <= x[i] + (y[i] >>> (i+1));
        y[i+1]   <= y[i] - (x[i] >>> (i+1));
    end
    else
    begin
        remainder[i+1] <= remainder[i] - angles[i];
        x[i+1]   <= x[i] - (y[i] >>> (i+1));
        y[i+1]   <= y[i] + (x[i] >>> (i+1));
    end
    end
end

always @(posedge clk)
begin
case (angle[N-1:N-2])
2'b00:
begin
x[0] <= 20'b01001001001111100000;
y[0] <= 20'b01001001001111100000;
remainder[0] <= angle - 20'b00110000110101000000;
end

2'b01:
begin
x[0] <= ~20'b01001001001111100000 + 1'b1;
y[0] <= 20'b01001001001111100000;
remainder[0] <= angle - 20'b00110000110101000000;
end

2'b10:
begin
x[0] <= ~20'b01001001001111100000 + 1'b1;
y[0] <= ~20'b01001001001111100000 + 1'b1;
remainder[0] <= angle - 20'hA0000;
end

2'b11:
begin
x[0] <= 20'b01001001001111100000;
y[0] <= ~20'b01001001001111100000 + 1'b1;
remainder[0] <= angle - 20'hE0000;
end
endcase

end
endmodule


[BrianHG]

Note that I have not analyzed your code, but when I usually create arrays in verilog, I usually assign the array size from [0:15].  I do not think there should be a difference having the 2 numbers backwards, but it is just a tiny thing I noticed...  If I remember, the only difference is the direction when you address the array.

EG:

Code: [Select]

...
wire [N-1:0] angles [0:15];
...
reg signed [N-1:0] remainder [0:16];
reg signed [N-1:0] x [0:16];
reg signed [N-1:0] y [0:16];
...
I'm not sure you should be using the genvar and generate...  We typically would use that to generate 16 copies of instantiated code or instantiated modules which would need replication.  It is not necessary for assigning a parallel array of logic.  All you should need is:

Code: [Select]

    always @(posedge clk)
    begin
for (int i=0;i<16;i++) begin
    if (remainder[i] < 0)
    begin
        remainder[i+1] <= remainder[i] + angles[i];
        x[i+1]   <= x[i] + {{i{y[i][N-1]}},y[i][N-1:i]};
        y[i+1]   <= y[i] - {{i{x[i][N-1]}},x[i][N-1:i]};
    end
    else
    begin
        remainder[i+1] <= remainder[i] - angles[i];
        x[i+1]   <= x[i] - {{i{y[i][N-1]}},y[i][N-1:i]};
        y[i+1]   <= y[i] + {{i{x[i][N-1]}},x[i][N-1:i]};
    end
    end // for int i
    end // always clk

All this could have been placed inside the same always @(posedge clk) as your case statement.

Assigning 1 time data would also look different.

[miken]

Have you simulated the code? That is the first thing to do since it gives you a bird's eye view of everything.

I agree with BrianHG that putting the for loop inside the always is preferable stylistically.

[BrianHG]

Yup, setup modelsim.  The testbench would only be around 20 lines long and the vlog/vsim commands would only have around 1-2 arguments each to simulate this code.

[knight]

Note that I have not analyzed your code, but when I usually create arrays in verilog, I usually assign the array size from [0:15].  I do not think there should be a difference having the 2 numbers backwards, but it is just a tiny thing I noticed...  If I remember, the only difference is the direction when you address the array.

EG:

Code: [Select]

...
wire [N-1:0] angles [0:15];
...
reg signed [N-1:0] remainder [0:16];
reg signed [N-1:0] x [0:16];
reg signed [N-1:0] y [0:16];
...
I'm not sure you should be using the genvar and generate...  We typically would use that to generate 16 copies of instantiated code or instantiated modules which would need replication.  It is not necessary for assigning a parallel array of logic.  All you should need is:

Code: [Select]

    always @(posedge clk)
    begin
for (int i=0;i<16;i++) begin
    if (remainder[i] < 0)
    begin
        remainder[i+1] <= remainder[i] + angles[i];
        x[i+1]   <= x[i] + {{i{y[i][N-1]}},y[i][N-1:i]};
        y[i+1]   <= y[i] - {{i{x[i][N-1]}},x[i][N-1:i]};
    end
    else
    begin
        remainder[i+1] <= remainder[i] - angles[i];
        x[i+1]   <= x[i] - {{i{y[i][N-1]}},y[i][N-1:i]};
        y[i+1]   <= y[i] + {{i{x[i][N-1]}},x[i][N-1:i]};
    end
    end // for int i
    end // always clk

All this could have been placed inside the same always @(posedge clk) as your case statement.

Assigning 1 time data would also look different.

Actually, I think we have to use generate. We cannot just use for loop like that. It gives error that "i" is not a constant.

[knight]

Have you simulated the code? That is the first thing to do since it gives you a bird's eye view of everything.

I agree with BrianHG that putting the for loop inside the always is preferable stylistically.

Yes, this is the output I get.

[miken]

Great, now using the sim you can select signals internal to your module to view. Keep following stuff back until you see something that looks correct; for example your sin should be the same as x[15], x[15] depends on x[14], etc. If nothing looks right then start from the beginning and see where things start to go wrong.

You are using i to do repetition, so I guess it's true that Verilog doesn't like that to not be a constant... But you are using it to manually do sign extension? That is kind of ugly. I haven't done a lot of arithmetic in Verilog (mostly VHDL) but I believe that as long as everything in the assignment expression is declared/casted signed, the sign extension will be automatically done.

[knight]

Great, now using the sim you can select signals internal to your module to view. Keep following stuff back until you see something that looks correct; for example your sin should be the same as x[15], x[15] depends on x[14], etc. If nothing looks right then start from the beginning and see where things start to go wrong.

You are using i to do repetition, so I guess it's true that Verilog doesn't like that to not be a constant... But you are using it to manually do sign extension? That is kind of ugly. I haven't done a lot of arithmetic in Verilog (mostly VHDL) but I believe that as long as everything in the assignment expression is declared/casted signed, the sign extension will be automatically done.

I tried with signed/unsigned but same output

[knight]

Great, now using the sim you can select signals internal to your module to view. Keep following stuff back until you see something that looks correct; for example your sin should be the same as x[15], x[15] depends on x[14], etc. If nothing looks right then start from the beginning and see where things start to go wrong.

You are using i to do repetition, so I guess it's true that Verilog doesn't like that to not be a constant... But you are using it to manually do sign extension? That is kind of ugly. I haven't done a lot of arithmetic in Verilog (mostly VHDL) but I believe that as long as everything in the assignment expression is declared/casted signed, the sign extension will be automatically done.

I think I'm getting close, Please see the updated code and the output.

[BrianHG]

I simed your attached code, but I am getting something different:



I also attached my modelsim setup & run .do batch files with tb to run the sim.

(EDIT: In the .do scripts, I forgot to set the analog height for the 'sine' output, just click on the waveform and select 'Format-Analog Automatic' to fix the waveform height.)

[BrianHG]

Just so you know, your Verilog code doesn't match the function of your VHDL code.
It is also the reason why your get the error ' error that "i" is not a constant.' when you remove the genvar and generate.
Also, without power-up defaults set for your regs, your code only generates red traces in the sim.  (My attached modelsim version shows you how to set your power-up defaults.)

[knight]

Just so you know, your Verilog code doesn't match the function of your VHDL code.
It is also the reason why your get the error ' error that "i" is not a constant.' when you remove the genvar and generate.
Also, without power-up defaults set for your regs, your code only generates red traces in the sim.  (My attached modelsim version shows you how to set your power-up defaults.)

((
I don't know any other way to use for loop

[BrianHG]

In your loop, shouldn't this:

Code: [Select]

                   x(i+1)   <= x(i) + y(i)(19 downto i+1);
                   y(i+1)   <= y(i) - x(i)(19 downto i+1);
....
                   x(i+1)   <= x(i) - y(i)(19 downto i+1);
                   y(i+1)   <= y(i) + x(i)(19 downto i+1);

have translated to this?

Code: [Select]

                   x[i+1]   <= x[i] + y[i][19 : (i+1)];
                   y[i+1]   <= y[i] - x[i][19 : (i+1)];
...
                   x[i+1]   <= x[i] - y[i][19 : (i+1)];
                   y[i+1]   <= y[i] + x[i][19 : (i+1)];

Or this?

Code: [Select]

                   x[i+1]   <= x[i] + (y[i] >> (i+1));
                   y[i+1]   <= y[i] - (x[i] >> (i+1));
...
                   x[i+1]   <= x[i] - (y[i] >> (i+1));
                   y[i+1]   <= y[i] + (x[i] >> (i+1));

These 2 examples should be better compatible with a for loop without having to use the genvar, if not, my second example should pass.

[BrianHG]

Note I had a typo above mistakenly using the  ' -: '  instead of the normal ' : '.

Yes, that first example of the 2 I provided might still require the 'genvar' since we are defining a variable bit range.  My second example with the bit shifting allows variable offsets from an integer.

[BrianHG]

Also, another thing, shouldn't the reg remainder be set to 'reg signed'?

After all, you are checking to see if it is less than '0'.


Hmmm, if X&Y are also signed, then I can see a problem with blindly bit shifting.  I know there is a cheap easy stupid simple way to fix that type of a problem.  If you need that, I will take a look later tonight when I wake up.  You can always shift your math to the mid-point of 20 bits and do everything un-signed.

[knight]

Also, another thing, shouldn't the reg remainder be set to 'reg signed'?

After all, you are checking to see if it is less than '0'.


Hmmm, if X&Y are also signed, then I can see a problem with blindly bit shifting.  I know there is a cheap easy stupid simple way to fix that type of a problem.  If you need that, I will take a look later tonight when I wake up.  You can always shift your math to the mid-point of 20 bits and do everything un-signed.

Ok. I changed it to shift operation and now no need of generate. Also, all the required signals are signed now, as in the original VHDL code. Now I'm getting a perfect triangular wave output. 

[BrianHG]

Here is how you should have setup your simulation:



As you can see, I'm running the VHDL and your Verilog in parallel.  The red arrows give you an idea of how to you can trace the differences in a simulator when setup like this.  I've attached the Modelsim project for you.  Since you haven't provided updates, it has your original code from post 1.

If I have time later tonight, I'll take a closer look and properly convert/interpret the VHDL into verilog.
The setup and run .do script files contain my example vlog / vcom / vsim commands necessary for running sims with mixed HDL code.

[BrianHG]

Ok, I did a proper translation.  Here are the results:



As you can see, the Verilog now matches the VHDL perfectly even with random angle inputs on every clock.  I changed the source files to SystemVerilog, but kept Verilog compatibility by using the old style 'reg' and 'always'.  As you can see in my code, there are no fancy tricks, not generate/genvar, and everything respects 'signed' logic within the core.  All unused bits have been shaved.  I found this to be an interesting endeavour as it is my first attempt at translating VHDL to Verilog.

Full source, simulation testbench and setup .do script for ModelSim included in the attached .zip file.

[BrianHG]

For those of you who do not wish to download my above attached .zip file, here:

Code: [Select]

// *********************************************************************
// BrianHG's Experimental conversion of sin_cos.vhd
// into System Verilog.
// April 20, 2022
// *********************************************************************

parameter N = 20;

module sin_cos_bg (
   input               clk    ,
   input               de_in  ,
   input       [N-1:0] angle  ,
   output wire         de_out ,
   output wire [N-1:0] sine   ,
   output wire [N-1:0] cosine  );

const integer angles [0:15] = '{77376, 40884, 20753, 10417,
                                 5213,  2607,  1304,   652,
                                  326,   163,    81,    41,
                                   20,    10,     5,     3};

reg                de_sr     [0:16] = '{default:0};
reg signed [N-1:0] remainder [0:16] = '{default:0};
reg signed [N-1:0] x         [0:16] = '{default:0};
reg signed [N-1:0] y         [0:16] = '{default:0};

assign de_out = de_sr[16] ;
assign sine   = x    [16] ;
assign cosine = y    [16] ;

always @(posedge clk) begin

        for (int i=0;i<=15;i=i+1) begin

               de_sr[i+1] <= de_sr[i];
               if (remainder[i] < 0) begin
                   remainder[i+1] <= remainder[i] + angles[i];
                   x[i+1]   <= x[i] + (y[i] >>> (i+1)); // >>> instead of >> to keep the original sign of the x/y[i].
                   y[i+1]   <= y[i] - (x[i] >>> (i+1));
               end else begin
                   remainder[i+1] <= remainder[i] - angles[i];
                   x[i+1]   <= x[i] - (y[i] >>> (i+1));
                   y[i+1]   <= y[i] + (x[i] >>> (i+1));
               end

        end // for loop

de_sr[0] <= de_in;

    if (de_in) begin
   
       case (angle[N-1:N-2])
         2'b00   : begin
                         x[0] <=  (N)'( 300000);
                         y[0] <=  (N)'( 300000);
                         remainder[0] <= (N)'(angle - 'h20000) ;
                   end
         2'b01   : begin
                         x[0] <=  (N)'(-300000);
                         y[0] <=  (N)'( 300000);
                         remainder[0] <= (N)'(angle - 'h60000) ;
                   end
         2'b10   : begin
                         x[0] <=  (N)'(-300000);
                         y[0] <=  (N)'(-300000);
                         remainder[0] <= (N)'(angle - 'hA0000) ;
                   end
         default : begin
                         x[0] <=  (N)'( 300000);
                         y[0] <=  (N)'(-300000);
                         remainder[0] <= (N)'(angle - 'hE0000) ;
                   end
       endcase
   
    end // if de_in

end // always clk
endmodule


[hamster_nz]

Nice work!

Might be worth  a comment that those magic numbers are

Code: [Select]

atan(0.5)/(2^pi)*(2^20),
atan(0.25)/(2*pi)*(2^20);
...

Just in case somebody wants to adapt it to their needs.

[knight]

Nice work!

Might be worth  a comment that those magic numbers are

Code: [Select]

arctan(0.5)/(2^pi)*(2^20),
arctan(0.25)/(2*pi)*(2^20);
...

Just in case somebody wants to adapt it to their needs.

Thanks!