Function result not locally static in case expression

Mon Nov 28, 2011 11:36 am   

Hello,

I wrote the following helper function to decode a chip select for
addressing a particular register in custom processor peripherals.

function CE_decode(CE_bit,CE_width : integer) return
std_logic_vector is
variable v_tmp : std_logic_vector(0 to CE_width-1) := (others =>
'0');
begin
v_tmp(CE_bit) := '1';
return v_tmp;
end function CE_decode;

The problem is that Xilinx XST returns a warning:
WARNING:HDLParsers:817 Choice CE_decode is not a locally static
expression.

I confirmed that the warning is actually redundant but I would still
like to get rid of it.

I am using the function in a context like this:

C_NUM_REG : integer := 4;

p_slv_reg_write : process(Bus2IP_Clk) is
begin
if rising_edge(Bus2IP_Clk) then
case slv_reg_write_sel is
when CE_decode(0, C_NUM_REG) =>
slv_reg0 <= Bus2IP_Data;
when CE_decode(1, C_NUM_REG) =>
slv_reg1 <= Bus2IP_Data;
when CE_decode(2, C_NUM_REG) =>
slv_reg2 <= Bus2IP_Data;
when CE_decode(3, C_NUM_REG) =>
slv_reg3 <= Bus2IP_Data;
when others => null;
end case;
end if;
end process p_slv_reg_write;

Instead of using the CE_decode function I can of course just write a
static expression,
but this gets hard to maintain and needs modification each time the
number of registers
in the peripheral changes. And it is hard to spot a mistake if there
are 32 registers
which CE is actually being decoded, e.g.
"00000000000000000010000000000000".

Can somebody please suggest how to work around the warning or suggest
a better way
in place of CE_decode function?

Thank you!

Mon Nov 28, 2011 12:29 pm   

constant ce_slv_reg0_c : std_logic_vector := CE_decode(0, C_NUM_REG);
constant ce_slv_reg1_c : std_logic_vector := CE_decode(1, C_NUM_REG);
constant ce_slv_reg2_c : std_logic_vector := CE_decode(2, C_NUM_REG);
constant ce_slv_reg3_c : std_logic_vector := CE_decode(3, C_NUM_REG);

They should be static enough for VHDL.

regards,
Bart

Mon Nov 28, 2011 1:01 pm   

On 28.11.2011 11:36, a s wrote:


Normally, breaking vec := (CE_BIT => '1', others => '0') into your code
solves the problem. I would first ensure that args are constants:

constant CE_bit,CE_width : integer

Mon Nov 28, 2011 1:24 pm   

a s <nospamas_at_gmail.com> wrote:

You could use if-constructs like this:

p_slv_reg_write : process(Bus2IP_Clk) is
begin
if rising_edge(Bus2IP_Clk) then
if slv_reg_write_sel = CE_decode(0, C_NUM_REG) then
slv_reg0 <= Bus2IP_Data;
end if;
if slv_reg_write_sel = CE_decode(1, C_NUM_REG) then
slv_reg1 <= Bus2IP_Data;
end if;
if slv_reg_write_sel = CE_decode(2, C_NUM_REG) then
slv_reg2 <= Bus2IP_Data;
end if;
if slv_reg_write_sel = CE_decode(3, C_NUM_REG) then
slv_reg3 <= Bus2IP_Data;
end if;
end if;
end process p_slv_reg_write;

Note that 'elsif' has intentionally not been used to avoid priority
decoders and because CE_decode() already guarantees non-overlapping
conditions.

If you put slv_regX into an array, you can even use a for loop instead
of explicitly writing all those if's.

Enrik

Mon Nov 28, 2011 3:08 pm   

On Mon, 28 Nov 2011 01:36:18 -0800, a s wrote:


.... and using it as

your static expressions (constants) ... but another way is to realise the
purpose of the case statement is to evaluate once, and simply compare
against a static list (effectively a list of constants.

So another way is to write a new function

subtype reg_addr is natural range 0 to C_NUM_REG;
-- Use a subtype here so synthesis doesn't use 32-bit arithmetic!

function select_reg(selector : ) return reg_addr is
-- a simple exercise...

case select_reg(slv_reg_write_sel) is
when 0 => slv_reg0 <= Bus2IP_Data;
when 1 => slv_reg1 <= Bus2IP_Data;
when 2 => slv_reg2 <= Bus2IP_Data;
when 3 => slv_reg3 <= Bus2IP_Data;
when others => null;
end case;

-- Brian

Tue Nov 29, 2011 11:05 am   

Thank you all! These suggestions above are all very interesting.

I decided to go with Brian's suggestions and implemented the
following function. I checked it in simulation and synthesis
and the function passed without warnings. Anyway, I would
appreciate to receive a comment if someone spots a weak point.

function select_reg(selector : std_logic_vector;
C_NUM_REG : integer)
return natural is
subtype reg_addr_t is natural range 0 to C_NUM_REG-1;
variable reg_addr : reg_addr_t := 0;
begin
for i in 0 to C_NUM_REG-1 loop
if selector(i) = '1' then
reg_addr := i;
exit;
end if;
end loop;
return reg_addr;
end function select_reg;

Thanks again!

Tue Nov 29, 2011 8:16 pm   

An alternative would be to define the registers as an array of vectors and
index the array directly:-

architecture ... of ... is

type register_file is array (range 0 to C_NUM_REG) of std_logic_vector(...);
signal slv_reg : register_file ;

begin

p_slv_reg_write : process(Bus2IP_Clk) is
begin
if rising_edge(Bus2IP_Clk) then
slv_reg(slv_reg_write_sel) <= Bus2IP_Data;
end if;
end process p_slv_reg_write;

end architecture ....


Then just use slv_reg(0) in place of slv_reg0 etc.

Tue Nov 29, 2011 8:55 pm   

Why are you passing c_num_reg to the function? The 'range attribute
from the selector argument will give you what you want:

function select_reg(selector : std_logic_vector) return natural is
variable reg_addr : natural range selector'range;
begin
for i in selector'range loop
if selector(i) = '1' then
reg_addr := i;
exit;
end if;
end loop;
return reg_addr;
end function select_reg;

Note that the function returns the same value whether no bits or set
or selector'low is set. May not be a problem in your application.

If you need to encode only a slice of a vector, then call the function
with the slice instead of the whole vector.

Andy

Wed Nov 30, 2011 8:26 am   

On 11/28/2011 1:36 AM, a s wrote:


Selects are a stubborn problem.
My last looked something like:

type cs_t is (ram, rom, led, keys, io);
type cs_bits_t is array (cs_t) of std_ulogic;
type cs_bytes_t is array (cs_t) of byte_t;
signal cs_s : cs_t;
signal cs_bits_s : cs_bits_t;
signal rdy_bits_s : cs_bits_t;
signal data_bytes_s : cs_bytes_t;
-- some limits record
-- some table of adr limit records:
type cs_table_t is array (cs_t) of cs_limits_t;
function cs_all return cs_bits_t is ...for loop for all selects
cs_s <= cs_now(cs_bits_s);
d <= data_bytes_s(cs_s);


-- Mike Treseler