DFiantHDL / DFHDL

DFiant HDL (DFHDL): A Dataflow Hardware Descripition Language
https://dfianthdl.github.io/
GNU Lesser General Public License v3.0
78 stars 8 forks source link

Incorrect conversion of signed SRA and std_logic_vector SRL for VHDL #118

Closed MartinC45 closed 5 months ago

MartinC45 commented 5 months ago

Description

Shifts are incorrectly converted to VHDL incorrectly in some cases. For a bitvector right shift sla is used instead of srl and for signed right shift the generated signed_sra function converts the signed value to unsigned, which leads to incorrect behavior for negative numbers. Shifts with unsigned (UInt) seem correct.

Example

//> using scala 3.4.0
//> using dep io.github.dfianthdl::dfhdl::0.4.3+12-a64c002c-SNAPSHOT
//> using plugin io.github.dfianthdl:::dfhdl-plugin:0.4.3+12-a64c002c-SNAPSHOT
//> using option -deprecation -language:implicitConversions

import dfhdl.*

class ShiftIssue() extends RTDesign:  
    val bitvec = Bits(10) <> VAR
    val bitvec2 = Bits(10) <> OUT
    val bitvec3 = Bits(10) <> VAR

    val signed1 = SInt(10) <> VAR
    val signed2 = SInt(10) <> OUT
    val signed3 = SInt(10) <> OUT

    val unsigned1 = UInt(10) <> VAR
    val unsigned2 = UInt(10) <> OUT
    val unsigned3 = UInt(10) <> OUT

    bitvec2 := bitvec >> 1 // not ok, uses sla instead of srl
    bitvec3 := bitvec << 1 // ok

    signed2 := signed1 >> 1 // not ok, converts value to unsigned before applying sra
    signed3 := signed1 << 1 // ok

    unsigned2 := unsigned1 >> 1
    unsigned3 := unsigned1 << 1 

given options.CompilerOptions.PrintGenFiles = true
given options.CompilerOptions.Backend = backends.vhdl

 @main def main = 
    ShiftIssue().compile

Output

library ieee;
use ieee.std_logic_1164.all;
use ieee.numeric_std.all;
use work.ShiftIssue_pkg.all;

entity ShiftIssue is
port (
  bitvec2   : out std_logic_vector(9 downto 0);
  signed2   : out signed(9 downto 0);
  signed3   : out signed(9 downto 0);
  unsigned2 : out unsigned(9 downto 0);
  unsigned3 : out unsigned(9 downto 0)
);
end ShiftIssue;

architecture ShiftIssue_arch of ShiftIssue is
  signal bitvec    : std_logic_vector(9 downto 0);
  signal bitvec3   : std_logic_vector(9 downto 0);
  signal signed1   : signed(9 downto 0);
  signal unsigned1 : unsigned(9 downto 0);
begin
  process (all)
  begin
    bitvec2   <= slv_srl(bitvec, 4d"1");
    bitvec3   <= slv_sll(bitvec, 4d"1");
    signed2   <= signed_sra(signed1, 4d"1");
    signed3   <= signed1 sla 1;
    unsigned2 <= unsigned1 srl 1;
    unsigned3 <= unsigned1 sll 1;
  end process;
end ShiftIssue_arch;
-- only relevant parts 
function slv_sll(slv : std_logic_vector; num_shifts : unsigned) return std_logic_vector is
begin
    return to_slv(unsigned(slv) sll to_integer(num_shifts));
end;
function slv_srl(slv : std_logic_vector; num_shifts : unsigned) return std_logic_vector is
begin
    return to_slv(unsigned(slv) sla to_integer(num_shifts)); -- uses sla
end;
function signed_sra(A : signed; num_shifts : unsigned) return signed is
begin
    return signed(unsigned(A) sra to_integer(num_shifts)); -- converted to unsigned
end;
end package body ShiftIssue_pkg;
soronpo commented 5 months ago

The srl was a mistake. I'm not sure the conversion to unsigned was wrong, but fixed it. Thanks!