vhdl tips, how can I make this code parametric ?

[legacy]

bus_mm_device_n can be { 3..4..5 }
bus_mm_sel_size is 2**bus_mm_device_n, therefore it can be { 8..16..32 }

I need to decode an address into slices
in_bus_mm.address(31 downto 32-bus_mm_sel_size)

with
bus_mm_device_n can be=4, bus_mm_sel_size=16
I have the following

Code: [Select]

    if (in_bus_mm.enable = '1' ) then
      case in_bus_mm.address(31 downto 28) is
          when "0000" => device_sel <= "0000000000000001";
          when "0001" => device_sel <= "0000000000000010";
          when "0010" => device_sel <= "0000000000000100";
          when "0011" => device_sel <= "0000000000001000";
          when "0100" => device_sel <= "0000000000010000";
          when "0101" => device_sel <= "0000000000100000";       
          when "0110" => device_sel <= "0000000001000000";
          when "0111" => device_sel <= "0000000010000000";
          when "1000" => device_sel <= "0000000100000000";
          when "1001" => device_sel <= "0000001000000000";
          when "1010" => device_sel <= "0000010000000000";
          when "1011" => device_sel <= "0000100000000000";
          when "1100" => device_sel <= "0001000000000000";
          when "1101" => device_sel <= "0010000000000000";
          when "1110" => device_sel <= "0100000000000000";
          when "1111" => device_sel <= "1000000000000000";
          when others => device_sel <= "0000000000000000";
      end case;       
    end if;

is there a way to make this code parametric? I don't get it 

[azer]

Code: [Select]

        if (in_bus_mm.enable = '1' ) then
          device_sel <= (others => '0');
          device_sel(to_integer(unsigned(in_bus_mm.address(31 downto 32-2**bus_mm_device_n)))) <= '1';
        end if;

define device_sel as

Code: [Select]

signal device_sel : std_logic_vector(2**bus_mm_device_n - 1 downto 0);

bus_mm_device_n needs to be a generic natural.

[legacy]

@azer
excellent, ghdl & ISE like that code, they both compile, thanks 

Code: [Select]

compiling bus_mm_device_sel ... done


[legacy]

Code: [Select]

package bus_mm_def is

  constant bus_mm_sel_size      : positive := 4; --bit
  constant bus_mm_device_n      : positive := 2**bus_mm_sel_size; --devices
...


[legacy]

assumed you did "use ieee.numeric_std.all;"

Code: [Select]

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

that is the header of the module
I am using ghdl, and ISE v10.1
the target is a spartan3e
and I also need to support a spartan6

but I'd like make the code as portable and reusable as possible

[Kalvin]

if anything else fails, make a script taking the parameters generating the VHDL source. I know, it ain't pretty but sometimes it just works.

[nctnico]

This is typically solved in a VHDL function using the length of the incoming signals using a for loop. Note that you'll need to iterate through each output bit. Something like this (not syntax checked but should be close):
for i in 0 to ouput'length-1 loop
if to_integer(input)=i then output(i) <= '1'; else output(i)<='0'; end if;
end loop;

Note: use an unsigned type for the input.

[legacy]

This is typically solved in a VHDL function using the length of the incoming signals using a for loop. Note that you'll need to iterate through each output bit. Something like this (not syntax checked but should be close):
for i in 0 to ouput'length-1 loop
if to_integer(input)=i then output(i) <= '1'; else output(i)<='0'; end if;
end loop;

Note: use an unsigned type for the input.

thanks, this idea looks interesting 

also it suggests me the following, in order to solve another problem that I have

Code: [Select]

  --BUS multiplexing out   
  process
  begin
  for i in 0 to bus_mm_device_n-1 loop
    if (device_sel(i) = '1') then
        bus_mm_out <= device_out(i);
      end if;   
  end loop;
  end process;           

  --BUS multiplexing in
  process
  begin
  for i in 0 to bus_mm_device_n-1 loop
    if (device_sel(i) = '1') then
        device_in(i) <= bus_mm_in;
      end if;   
  end loop;
  end process;


[nctnico]

Your code above infers latches so it is not good.
Just use:
bus_mm_out <= device_out(to_integer(in_bus_mm.address(31 downto 28)));

It is a good custom to use the signed and unsigned type for anything which represents a number or index (an address is an index!).

I don't understand why you want to multiplex the incoming bus though. Just connect the incoming bus to all devices and use device_sel to make a device do something with the data.

[AndyC_772]

This is typically solved in a VHDL function using the length of the incoming signals using a for loop. Note that you'll need to iterate through each output bit. Something like this (not syntax checked but should be close):
for i in 0 to ouput'length-1 loop
if to_integer(input)=i then output(i) <= '1'; else output(i)<='0'; end if;
end loop;

Another option is to take advantage of the fact that later assignments take priority over earlier ones:

Code: [Select]

output <= (OTHERS => '0');
output (to_integer(input)) <= '1';

Add an 'if' statement to check for the bounds on the input if required. In this example, we know the input is a 4 bit value, so it can only ever take values that correspond to 0-15. The case in which all output bits remain zero can't actually happen, so the check is redundant.

Note for anyone unfamiliar with VHDL: this code doesn't result in all the bits being driven low, and *then* one of them going high. It's not executed sequentially like a traditional language being executed on a microprocessor. Think of it as meaning "all the outputs go low... *except* for the one which needs to go high.

[legacy]

I don't understand why you want to multiplex the incoming bus though. Just connect the incoming bus to all devices and use device_sel to make a device do something with the data.

currently(1) I don't actually need the multiplex-in at all, since I can propagate the bus_mm_in to in-port of all sub-modules, it's not a problem and works fine, whereas when you have a lot of modules (devices attached to a bus) the real problem is that the out-port of each module which can't collide, therefore you need to connect them to the bus trough a multiplex



(1) I don't have a cross-matrix switch in my design, devices are connected to the bus_mm trough a multiplexer, the model is ... one input-one output, whereas the cross-bar matrix allows multiple input- multiple output, but it adds a lot of complexity and I don't want to implement it

in short, the above code was written to master vhdl deeply, I actually need just the multiplex-out part 

[nctnico]

Another option is to take advantage of the fact that later assignments take priority over earlier ones:

It is an option as well but I'm not a fan of VHDL code which (silently) relies on the order of the assignments because it is not a well known feature. Personally I prefer code which clearly tells what it does. Also the next engineer may swap or remove the lines because 'everything in VHDL is parallel anyway'.

[AndyC_772]

I felt much the same way at first, but there are some use cases that changed my mind completely.

One was a piece of code which had to take data from a source and, each time a new word was available, clock it into a FIFO.

In this case, it's important that the FIFO write enable is only activated for a single clock edge, and only when a new word has actually become available. It must not be left active under, say, error or exception conditions.

Writing:

Code: [Select]

fifo_we <= '0'

IF (string of conditions which indicate the new data is valid)
  fifo_we <= '1';
END IF;

... is both clear and resilient. It's immediately apparent by inspection that fifo_we will *always* be assigned a new value, and that it will only ever take the value '1' under the correct set of conditions. It will never be left at an old value, which once in a blue moon will be a '1' and will cause a FIFO overflow.

[legacy]

Code: [Select]

      for i in 0 to device_sel'length-1 loop
        if to_integer(unsigned(in_bus_mm.address(31 downto 32-bus_mm_device_n)))=i then
          device_sel(i) <= '1';
        else
          device_sel(i)<='0';
        end if;
      end loop;


Code: [Select]

  --BUS multiplexing out
    bus_mm_out <= device_out(to_integer(unsigned(in_bus_mm.address(31 downto 32-bus_mm_sel_size))));

perfect, both ghdl and ISE accept the code, also the test bench passes the test
code: approved 

Code: [Select]

device_01 : bus_mm_device_generic
    generic map
    (           
      clock_frequency      => clock_frequency,
      name                 => "device01",
      id                   => 01
    )
    port map
    (
      in_clock             => in_clock,
      in_reset             => in_reset,
      in_enable            => device_sel(1),
      in_bus_mm            => device_in(1),
      out_bus_mm           => device_out(1)
    );
-------------------------------------------------------------------------------------------
  device_02 : bus_mm_device_controller_sram_asynchronous
    generic map
    (           
      clock_frequency      => clock_frequency,
      name                 => "ctrl_asram",
      id                   => 02
    )
    port map
    (
      in_clock             => in_clock,
      in_reset             => in_reset,
      in_enable            => device_sel(2),
      in_bus_mm            => device_in(2),
      out_bus_mm           => device_out(2),
      --------------[phy]--------------------   
      in_phy               => in_devices_phy.device_02,
      out_phy              => out_devices_phy.device_02   
    );

it will be used as backplane in order to connect devices like the above


as you have suggested,

Code: [Select]

--device 00
      in_bus_mm            => device_in(0), <----- it needs a multiplex-in
--device 01
      in_bus_mm            => device_in(1), <----- it needs a multiplex-in
--device 15
      in_bus_mm            => device_in(15), <----- it needs a multiplex-in

can be simplified to

Code: [Select]

--device 00
      in_bus_mm            => device_in
--device 01
      in_bus_mm            => device_in
--device 15
      in_bus_mm            => device_in


[hamster_nz]

What makes you think that your code is sub-optimal? It looks like a good solution to the problem of building a memory address decoder.... and it will allow you to split the address ranges if you want, which is a good thing as hardware isn't always as clean as software - and after all, a different implementation won't be any more efficient. Your implementation is explicit, can be audited, doesn't rely on hidden, implicit behavior, it just lacks a little bit of flexibility.

Your code might be expressed a little cleaner without the long constants, which can be done like this:

Code: [Select]

process(in_bus_mm.address)
   begin
      -- default to zeros
      device_sel <= (others => '0');
      -- set whichever bit is needed
      case in_bus_mm.address(31 downto 28) is
          when "0000" => device_sel(0) <= in_bus_mm.enable;
          when "0001" => device_sel(1) <= in_bus_mm.enable;
          when "0010" => device_sel(2) <= in_bus_mm.enable;
     ....
     end case;
  end process;


However, you might be feeling uneasy because you know you are looking at the problem from the wrong perspective, and perhaps your subconscious is struggling to let you know what you really should be doing.

Most likely the solution you are wanting to find is this:

Code: [Select]

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

entity addr_decode is
    Port ( addr_in : in  STD_LOGIC_VECTOR;
           enable  : in  STD_LOGIC;
           dev_sel : out STD_LOGIC_VECTOR);
end addr_decode;

architecture Behavioral of addr_decode is

begin

g: for i in 0 to dev_sel'high generate
      dev_sel(i) <= enable when unsigned(addr_in) = to_unsigned(i, addr_in'length) else '0';
   end generate;

end Behavioral;

As addr_in is an unconstrained vector it can be as long or as short as you want. you just need to pass in the bits you want to decode (e.g. "addr_in => address(15 downto 12)" when you create an instance)

As dev_sel is also an unconstrained vector, you can decode as many dev_sel lines as you want, as long as they start at zero and are contiguous. If you don't use them all in your design they will be optimized away.

The only slight worries are if you as for more dev_sel outputs than the addr_in can encode strange things may happen. Hopefully "to_unsigned(i, addr_in'length)" will through an error if 'i' can not be decoded in "addr_in'length" bits - but at least in ISE it doesn't, it only decodes the first 8 dev_sel bits, the rest are just zeros.

It can be used like this - decoding four bits into 12 select lines:

Code: [Select]


library IEEE;
use IEEE.STD_LOGIC_1164.ALL;

entity test_addr_decode is
    Port ( addr_in : in  STD_LOGIC_VECTOR (3 downto 0);
           enable  : in  STD_LOGIC;
           dev_sel : out STD_LOGIC_VECTOR (11 downto 0));
end test_addr_decode;

architecture Behavioral of test_addr_decode is
   COMPONENT addr_decode
   PORT(
      addr_in : IN  STD_LOGIC_VECTOR;         
      enable  : IN  STD_LOGIC;
      dev_sel : OUT STD_LOGIC_VECTOR
   );
   END COMPONENT;
begin

i_addr_decode: addr_decode PORT MAP(
    addr_in => addr_in,
    enable  => enable,
    dev_sel => dev_sel);

end Behavioral;

On the surface it appears to work, but I don't like it, it has lurking bugs like simulation mismatches.

To prove a point I simulated the above code in ISM, and with three bits of address and 12 bits of dev_sel, An input of "011" would give "000000001000" in hardware (as checked in the technology schematic). In Simulation it gives "100000001000", as to_integer(11,3) apparently results in a value of 3.

All these different methods result in the exact same logic, so are much a muchness to me... stick with whichever you like. But being too smart is dumb.

[legacy]

Code: [Select]

entity addr_decode is
    Port ( addr_in : in  STD_LOGIC_VECTOR;


your idea looks brilliant, thanks very much 

just a note, it seems that *ghdl* doesn't like "opaque" types

Code: [Select]

.//decoder.vhd:15:1: range must be a static discrete range

it claims an error at line15 because it is expecting an explicit size

Code: [Select]

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

entity addr_decode is
    port
    (
      in_addr     : in  decoder_addr_t;
      in_enable   : in  std_logic;
      out_dev_sel : out decoder_sel_t
    );
end addr_decode;

architecture behavioral of addr_decode is

  constant msb: positive:=(bda_decoder_address_width - 1);
  constant lsb: positive:=(msb + 1 - bda_decoder_sel_size);

begin

  bda_decoder: 
  for i in 0 to bda_decoder_device_n-1 generate
      --out_bda_decoder'high
      out_device_sel(i) <=
        in_enable when unsigned(in_address(msb downto lsb))
                  = to_unsigned(i, bda_decoder_address_width)
                  --in_address'length
        else '0';
  end generate;

end behavioral;

I have adapted your code, sizes are defined inside a package, and even the test bench is fine 

[rstofer]

Another option is to take advantage of the fact that later assignments take priority over earlier ones:

It is an option as well but I'm not a fan of VHDL code which (silently) relies on the order of the assignments because it is not a well known feature. Personally I prefer code which clearly tells what it does. Also the next engineer may swap or remove the lines because 'everything in VHDL is parallel anyway'.

Having default values at the beginning of an FSM seems to be required in VHDL unless I want to provide an assignment for every single signal in every single state.  That's just too ugly to contemplate when an entire CPU is one big FSM with several dozen states and a couple of dozen output signals.

If I don't provide the default values, latches will be inferred for every signal which is not assigned a value in every state.  In fact, I have found that when my code blows up, more often than not it is because I failed to provide a default value and the synthesizer created a latch to store some previous assignment - probably not the value I would have used as a default.

Code: [Select]

[font=courier]
process (state, FullEA, FetchOpnd, F, TAG, IA, CO, OFL, OVFLInd, COtemp, CSET, VSET,
             r_Button0, CCC, CondMet, BOSC_Flag, SavedSign, A_BUS(15), ShiftCount,
            SZ, ZR, DVDS, Result, Ones, OVR,
            CountShifts, ACC, IncludeEXT, EXTN, Rotate, AFR,
            BitCount, XIO_Device, XIO_Function, XIO_Modifier,
            DisplaySwitch,
            ConsoleXIOCmdBusy, ConsoleXIOCmdAck,
            PrinterXIOCmdAck, PrinterXIOCmdBusy,
            ReaderXIOCmdBusy, ReaderXIOCmdAck,
            DiskXIOCmdBusy, DiskXIOCmdAck,
            DiskReady, IAR,
            SingleStep, BreakPointActive, BreakPoint,
            PendingInterrupt, ReturnState_r, StartState,
            ColdStartHold,
            PlotterXIOCmdBusy, PlotterXIOCmdAck,
            DISP, CPUDataIn, CPUAck) is
begin
A_BusCtrl                <= A_BUS_NOP;
ACC_Ctrl                 <= ACC_NOP;
ACC_ShiftIn            <= '0';
Add                        <= '0';
AFR_Ctrl                 <= AFR_NOP;
BitCountCtrl            <= BitCount_NOP;
CI <= '0';
CIn                        <= '0';
CIX                        <= '0';
CarryIndCtrl           <= CARRY_IND_NOP;
ConditionEnable      <= '0';
ConsoleXIOCmdReq <= '0';
COtempEnable        <= '0';
DGX                       <= '1'; -- normally pass AFR through AFR_MUX;
DZR                       <= '0';
DOVR                     <= '0';
DiskXIOCmdReq      <= '0';
EXT_Ctrl                 <= EXT_NOP;
EXT_ShiftIn             <= '0';
IAR_Ctrl                  <= IAR_NOP;
IR_Ctrl                    <= IR_NOP;
ISL_Ctrl                   <= ISL_NOP;
LoadConsoleXIOCmd <= '0';
LoadDiskXIOCmd      <= '0';
LoadDVDS                <= '0';
LoadPlotterXIOCmd   <= '0';
LoadPrinterXIOCmd   <= '0';
LoadReaderXIOCmd   <= '0';
LoadTAR                   <= '0';
NextState                 <= Fetch;
OvflIndCtrl                <= OVFL_IND_NOP;
PA_Ctrl                     <= PA_NOP;
PlotterXIOCmdReq     <= '0';
PrinterXIOCmdReq     <= '0';
RamRd                      <= '0';
RamWr                      <= '0';
ReaderXIOCmdReq     <= '0';
SAR_Ctrl                    <= SAR_NOP;
SaveSign                   <= '0';
ShiftCountCtrl            <= SHIFT_COUNT_NOP;
Subtract                    <= '0';
Trigger                      <= '0';
XIO_Ctrl                   <= XIO_NOP;
ReturnState              <= Fetch;
LatchReturnState       <= '0';
CPUReq                     <= '0';
CPUWr                       <= '0';
LatchCPUDataOut        <= '0';
LatchCPUAddr              <= '0';

case state is
when s0 => NextState <= s0a; -- use this to IPL
when s0a => if DiskReady = '0' then -- wait for disk to go not ready

At the beginning is a rather extensive sensitivity list  (to keep the synthesizer from whining) followed by the default assignments for every signal the FSM will manipulate (to prevent latches from being inferred) followed finally by the first two states (of dozens).


Legacy:  I would have written the decoder exactly the way you did in your OP.  I want my code to tell a story, an obvious story, that doesn't require me to have to take up a pencil and paper to figure out what I did.

It's worthwhile to look at the RTL schematic after synthesis and see how much logic the various approaches generated.  Maybe look at the resource usage as well.

[nctnico]

@rstofer: it is a good habit to put asynchronous logic like that not in a process but in a function. Especially when targetting an FPGA it is a good rule of thumb to only have a clock in the sensitivity list (and maybe an asynchronous reset if the FPGA supports it and doesn't eat routing resources).

[rstofer]

@rstofer: it is a good habit to put asynchronous logic like that not in a process but in a function. Especially when targetting an FPGA it is a good rule of thumb to only have a clock in the sensitivity list (and maybe an asynchronous reset if the FPGA supports it and doesn't eat routing resources).

That particular process isn't clocked, there is a separate next-state process that is clocked.  Like you said, it is pretty simple with only clk, reset and next_state in the sensitivity list.

I have never written any code in a function.  I'll have to look into it because it is something else to learn but I had thought they were more for shared or reusable code.  My large FSM is a one-of-a-kind deal so I just wrote it as an unclocked process with the small clocked process handling the next_state.

The only reason for having a sensitivity list is to give the simulator something to work with.  If the synthesizer wouldn't whine, I wouldn't even bother with one as I don't tend to use the simulator.  Now that I'm using Vivado, I'll probably make a lot of use of the Integrated Logic Analyzer.  That's a pretty neat piece of IP.

[hamster_nz]

Firstly, never read comments that include with "If we assume a simple LUT6 architecture" - they are just not worth wasting the brain cells over.

With that said... in some cases, something like:

  bus_mm_out <= device_out(to_integer(in_bus_mm.address(31 downto 28)));

might not be the best way to implement that this structure - using an "Output enable" AND gate on the device output, and then just have a wide OR get to combine the signals for each output bit can be superior.

This is because the fan-out on some of the address select lines gets very high, as it has to drive lots of LUTs.

If we assume a simple LUT6 architecture and implement merging 64 16-bit into one 16-bit bus, the fanout on a couple of bits of the address select might be as high as 256. Moving to an "AND then OR" arrangement should keep the fanout at around that of the bus width, but introduce a couple more levels of logic.

1) It should also do wonders for routing congestion
2) Power usage should improve, as the inactive parts of the OR matrix will have no toggling going on
3) The 'output enable' AND gates can get pushed back closer to where the values are generated, with only the wide OR gate needed to be close together, as each of the eight 64-input OR gates isn't sharing control signals with any of the others (which is not the case if MUXes are used)

[legacy]

Code: [Select]

  bus_mm_out <=
              device_out(0)  when device_sel(0)  = '1' else
              device_out(1)  when device_sel(1)  = '1' else
              device_out(2)  when device_sel(2)  = '1' else
              device_out(3)  when device_sel(3)  = '1' else
              device_out(4)  when device_sel(4)  = '1' else
              device_out(5)  when device_sel(5)  = '1' else
              device_out(6)  when device_sel(6)  = '1' else
              device_out(7)  when device_sel(7)  = '1' else
              device_out(8)  when device_sel(8)  = '1' else
              device_out(9)  when device_sel(9)  = '1' else
              device_out(10) when device_sel(10) = '1' else
              device_out(11) when device_sel(11) = '1' else
              device_out(12) when device_sel(12) = '1' else
              device_out(13) when device_sel(13) = '1' else
              device_out(14) when device_sel(14) = '1' else
              device_out(15);

at the beginning I was using the above

[rstofer]

Code: [Select]

  bus_mm_out <=
              device_out(0)  when device_sel(0)  = '1' else
              device_out(1)  when device_sel(1)  = '1' else
              device_out(2)  when device_sel(2)  = '1' else
              device_out(3)  when device_sel(3)  = '1' else
              device_out(4)  when device_sel(4)  = '1' else
              device_out(5)  when device_sel(5)  = '1' else
              device_out(6)  when device_sel(6)  = '1' else
              device_out(7)  when device_sel(7)  = '1' else
              device_out(8)  when device_sel(8)  = '1' else
              device_out(9)  when device_sel(9)  = '1' else
              device_out(10) when device_sel(10) = '1' else
              device_out(11) when device_sel(11) = '1' else
              device_out(12) when device_sel(12) = '1' else
              device_out(13) when device_sel(13) = '1' else
              device_out(14) when device_sel(14) = '1' else
              device_out(15);

at the beginning I was using the above

What got synthesized?  I might not have used one-hot selection and would probably have used a 4 bit address select field and left it to the synthesizer to work out the details.  But the code would have looked the same.  It is, after all, a 16 input MUX and FPGA synthesizers know how to build that structure.  I might have coded device_out(15) with the 'when others' clause so that the syntheszer can clearly see that all states are covered.  I don't know that it matters but it's worth an experiment to find out.

I tend to like the case statement for this kind of thing.  On paper, as I read the listing, it tells a better looking story.

If you want to get right down to the LUTs, see page 17:

http://www.xilinx.com/support/documentation/application_notes/xapp522-mux-design-techniques.pdf

I'm not sure I want to have to write the bit patterns for slices.  Talk about obscure code!

Page 22 discusses your 1 or N selection approach and, without looking too close, it seems that it will result in more resources used versus a binary selection.

[legacy]

I need to have a parametric code, I mean the number of devices in the mux should be an input parameter defined in package, also the mutex is not multiplexing a simple signal, it multiplex a record, structured signal

Code: [Select]

  type bus_mm_out_t is
  record
    ...
  end record;


the following code is NOT a good idea, and it doesn't work in ghdl

Code: [Select]

  multiplex_out:
for i in 0 to bus_mm_device_n-1 generate
      bus_mm_out <= device_out(i) when device_sel(i)  = '1';
end generate;

whereas the following code works in ghdl, but it implies latches with ISE's synthesizer

Code: [Select]

  --BUS multiplexing out   
  process
  begin
  for i in 0 to bus_mm_device_n-1 loop
    if (device_sel(i) = '1') then
        bus_mm_out <= device_out(i);
      end if;   
  end loop;
  end process;


[legacy]

what do you think about a function approach ?

Code: [Select]

function mux_out
         (                         
           sel  : bus_mm_device_sel_t;
           pool : bus_mm_pool_out_t       
         )
         return bus_mm_out_t
is
  variable ans : bus_mm_out_t;
begin

  --What happens if sel() happens to be all 0's
  --Sure, you know that can't happen
  --but the synthesizer doesn't.   
  ans := pool(0);

  for i in 0 to bus_mm_device_n-1 loop
    if (sel(i) = '1') then
      ans := pool(i);
    end if;   
  end loop; 

  return ans;
end function;


edit:
there was a mistake
it needs to be      if (sel(i) = '1') then

edit:
modified according to the advice of providing a default value

[rstofer]

Try defining bus_mm_out for ALL conditions:

Code: [Select]

  --BUS multiplexing out   
  process
  begin

      bus_mm_out <= (others => '0');  -- add the default output

  for i in 0 to bus_mm_device_n-1 loop
    if (device_sel(i) = '1') then
        bus_mm_out <= device_out(i);
      end if;   
  end loop;
  end process;


I haven't tried this but when I wind up with latches it is ALWAYS because I haven't covered EVERY possible assignment possibility.  I'm not saying that all possibilities aren't covered, I'm saying the synthesizer doesn't KNOW they are all covered because that would require a deeper analysis of device_sel().

That's what I was showing yesterday with that great long set of default outputs.

[rstofer]

what do you think about a function approach ?

Code: [Select]

function mux_out
         (                         
           sel  : bus_mm_device_sel_t;
           pool : bus_mm_pool_out_t       
         )
         return bus_mm_out_t
is
  variable ans : bus_mm_out_t;
begin

  for i in 0 to bus_mm_device_n-1 loop
    if (sel(i) = '1') then
        ans := pool(0);
      end if;   
  end loop; 

  return ans;
end function;


The code probably should be a function or procedure because it is the kind of thing you might want in a library.  I'm no help with these choices.

But I'll bet you still get latches.  You just can't show that 'ans' is assigned an output for every possible input.  You can just assign a default value right after 'begin' (ans <= (others => '0'), assuming 'ans' is a vector).  What happens if sel() happens to be all 0's.  Sure, you know that can't happen but the synthesizer doesn't.

I really hope I'm firing on all thrusters!  Try my simple changes and see what happens.  If I'm all wrong, let me know!

As I understand latches, 'ans' will be assigned the value most recently 'latched' if  every 'if' statement is false.  But 'ans' WILL be assigned a value.  It's hardware...

I don't use the simulator and there is every chance that a simulation won't look like the value is latched.  I know that I have to be very careful to cover all output conditions where I actually run the hardware.

[legacy]

What happens if sel() happens to be all 0's.  Sure, you know that can't happen but the synthesizer doesn't.

ah, ok, now I have fully understood your point

[AndyC_772]

Not really on topic, but how can you see if code infers latches?
I usually have a process like:

Code: [Select]

process (clk) is
begin
    if rising_edge(clk) then
        <logic>
    end;

Which worked well enough for my purposes (but there are probably scenario's in which you want to do something else).

There's a difference between inferring a latch and inferring a clocked register bit. It's confusing that the term "latch" is ambiguous in this way.

If you have synchronous, clocked logic which doesn't update the state of an output under all conditions, then a D-type is inferred because it has to retain the state which was assigned to the output on a previous edge. This might be a small waste of logic, but it's not actually a problem.

So, for example, it's OK to write:

Code: [Select]

if rising_edge (clk) then
  if condition_a = '1' then
    output <= '1';
  else
    if condition_b = '1' then
      output <= '0';
    end if;
  end if;
end if;

This code sets the output high under condition 'a', and low under condition 'b'. If neither condition is true, then the output stays at its previous value, and a D-type is therefore needed to keep a record of what that value should be.

The problem comes if your logic isn't synchronous:

Code: [Select]

  if condition_a = '1' then
    output <= '1';
  else
    if condition_b = '1' then
      output <= '0';
    end if;
  end if;

This code is, of course, functionally similar, but it can't use a D-type because there's no clock. It still needs to maintain the value of the output during any times when both conditions are 0, and that infers a latch.

[legacy]

The problem comes if your logic isn't synchronous

that's the reason why I am doing everything synchronous, including the ALU modules
in the theory I could even rewrite the bus multiplexer to be synchronous
and it will cost an extra clock tick through the current cpu's data-path
 

[legacy]

ans <= (others => '0'), assuming 'ans' is a vector

it's an array of record (structured signals)

Code: [Select]

  type bus_mm_out_t is
  record
    data         : bus_mm_data_t;
    device_sel   : bus_mm_device_sel_t;
...
  end record;

bus_mm_data_t is a vector, with a size of 32bit

Code: [Select]

  type bus_mm_pool_out_t     is array (0 to bus_mm_device_n-1) of bus_mm_out_t;

this type represents the whole backplane as a collection of devices (excluding their physical signals, which are handled and propagated through another dedicated record-type)

Code: [Select]

-- [CPU] ==== core_bus ==== [adapter] ==== bus_mm
--
-- bus_mm
--    +-- mux
--          +-- device0
--          +-- device1
--               ...
--          +-- devicen-1

the core_bus is a simple cpu_bus which doesn't care about misalignment, dtack, wait-cycles, and other details, it simply passes the IO_request{rw,size,data,address} to the adapter which uses the multiplexer in order to *select* the proper device (16 devices are available at the moment), also the adapter handles all the low level detail (misalignment, dtack, wait-cycles, physical size-cycles e.g. a NVRAM of 8bit attached to a 32bit data bus, etc), and back propagates a feedback to the core_bus: { success, failure&cause }

the cpu doesn't care about those details, it's not like on a MIPS processor where you have to physically handle the misaligned access with a specific pair of instructions, it's more like the 68000 cpu

in case of failure an exception is also raised and the cop0 (exception handler) will look at the cause- register in order to issue the right ISR-vector to the cpu: bus error exception




not yet completed, it's based on an old working project I done 3 years ago, it's the first on the left in the picture, whereas the last one on the right is the current project, completely rewritten from the scratch, you can see a few progress 

[rstofer]

I haven't progressed to the point of using 'records' for data structures.  I can see where they can be used to encapsulate a group of signals and  next time I code my 1130, I'll try that for all of the IO devices.  They all work the same way:  They have a device id, the use the DMA channel for all transfers which implies an address bus, a REQ/GRANT arrangement and so on.  This could save me some states by eliminating code that  now talks to a particular IO device.

From the point of view of the DMA channel, all devices are equal in terms of hardware including the CPU.  There is a priority encoder scheme to keep the CPU from hogging but that detail is internal.

Yup!  Something else to study...

[rstofer]

At around 17 minutes in this video, there is a discussion about building MUXes and the best choice seems to be the CASE statement rather than the if-then-else which actually builds a priority tree.  I'm not sure what the 'when' arrangement creates but I wouldn't be surprised that it looks like the if-then-else situation.

In any event, this video was 47 minutes well spent.  There is quite a discussion on FSMs and pipelining:

http://www.xilinx.com/video/hardware/basic-hdl-coding-techniques.html

[legacy]

In any event, this video was 47 minutes well spent.  There is quite a discussion on FSMs and pipelining:
http://www.xilinx.com/video/hardware/basic-hdl-coding-techniques.html

it's always time well spent when it also offers an occasion to improve my English
thanks twice, it's also a very very nteresting video