Weirdness with uninferred RAM, VHDL(QuartusII).

[KaptenFrans]

Hi!
I'm currently taking a course in VHDL, and as a final project I'm supposed to design a very simple CPU. However I've run into some problems with my RAM. Until now I've only read from it, but now I'm trying to write to it instead. It's (supposed to be at least ) a 16bit wide, 8bit deep RAM with a single clock, dual-port, a write enable and some fluff to make it wishbone-compatible(Not there yet, I think). My code is down below.

When I read from it everything is fine and dandy. During compilation Quartus I get the following message:

Code: [Select]

Info (19000): Inferred 1 megafunctions from design logicand in Modelsim I can get my instructions out.

But as soon as I drive WE_I high(From my Control Unit, I do not touch the code for the RAM at all) to write, Quartus reports that:

Code: [Select]

Info (276014): Found 1 instances of uninferred RAM logic
Info (276007): RAM logic "RAM:b2v_RAM|RAM256" is uninferred due to asynchronous read logic

And also that

Code: [Select]

Warning (335093): TimeQuest Timing Analyzer is analyzing 32 combinational loops as latches.
Everything seem to work alright in Modelsim, but the compilation takes six times as long(From ~30s to 3min) which is not really optimal.  And most of all I want to understand why this is happening?
I do (sort of) understand what combinational loops are, but not how they can appear just by driving an existing signal high? Is this something that will bite me later if I don't fix it or can I safely ignore it?

My Control Unit is implemented as a state machine with two processes, one handling state changes and the other the outputs in each state. WE_I is driven low by 'default'.

Can anyone, please, help me solve this? I've been banging my head against the wall for close to 13h now. I'm designing for the DE2 dev-board with CycloneII FPGA.

Code: [Select]

entity RAM is
generic(
ADDR_WIDTH : integer := 8;
DATA_WIDTH : integer := 16
);
port(
CLK_I, RST_I, WE_I, STB_I : in std_logic;
ADR_I : in std_logic_vector(7 downto 0); -- Address in
DAT_I : in std_logic_vector(15 downto 0); -- Data in
SEL_I : in std_logic_vector(7 downto 0);  -- Wishbone fluff
ACK_O : out std_logic; -- ACK for wishbone
DAT_O : out std_logic_vector(15 downto 0) -- Data out
);
end entity RAM;

architecture RTL of RAM is

subtype WORD is std_logic_vector(15 downto 0);
type MEMORY is array(0 to 2**ADDR_WIDTH-1) of WORD;
signal RAM256:MEMORY := (
"0100000000001011", -- 0: LOAD 11 (Bit 15-13 determines the instruction)
"0010000000001100", -- 1: ADD 12
"0110000000001101", -- 2: STO 13
-- ... Cut out to save space ... --
"0000000000000001", -- 11: 1
"0000000000000010", -- 12: 2
others => "0000000000000000");
begin

process(CLK_I, RST_I, ADR_I, STB_I)
    variable RAM_ADDR_IN : integer range 0 to 2**ADDR_WIDTH-1;
begin
if (RST_I = '1') then
DAT_O <= (others => '0');
ACK_O <= '0';
elsif rising_edge(CLK_I) then
RAM_ADDR_IN := conv_integer(ADR_I); -- Convert the adress vector to int
ACK_O <= '0';
if STB_I = '1' then
if (WE_I = '1') then
RAM256(RAM_ADDR_IN) <= DAT_I; -- Write what's on the DAT_I to memory at RAM_ADDR_IN
else
DAT_O <= RAM256(RAM_ADDR_IN); -- Read what's in the memory and put it on DAT_O
end if;
ACK_O <= '1';
end if;
end if;
end process;
end rtl;

 

[Kalvin]

What happens if you remove ADR_I, STB_I from the process's sensitivity list?

You may want to see the Quartus II synthesis guides. For example:
http://quartushelp.altera.com/14.0/mergedProjects/hdl/vhdl/vhdl_pro_ram_inferred.htm

[AndyC_772]

This may have something to do with the fact that you have an asynchronous reset.

Quartus will try to recognise when your code can be most efficiently compiled using some of the hard RAM blocks that exist in the FPGA silicon. If the behaviour of your code matches that of the hard RAM block, then the RAM block can be used, which saves a load of general purpose logic cells.

Your RAM array is a prime candidate for implementation this way, of course.

However: your output DAT_O doesn't quite match the capabilities of the hard RAM blocks, which are fully synchronous - ie. every input and every output is clocked. DAT_O is forced low whenever RST_I is high, independently of your clock, which might explain the error "uninferred due to asynchronous read logic".

If WE_I is never driven high, then Quartus can optimise out the RAM entirely; it becomes a small ROM instead, which in turn might be getting implemented in logic cells rather than using a hard RAM block. The warnings go away because Quartus chose not to use a RAM block at all.

You might be having an issue with combinatorial loops because you're trying to force DAT_O to be something other than the RAM contents when the reset signal is asserted. That means that DAT_O cannot be the same as the usual signal output from a RAM block, because the multiplexer you'd need doesn't exist.

Try using a synchronous reset and see if that helps:

Code: [Select]

entity RAM is
generic(
ADDR_WIDTH : integer := 8;
DATA_WIDTH : integer := 16
);
port(
CLK_I, RST_I, WE_I, STB_I : in std_logic;
ADR_I : in std_logic_vector(7 downto 0); -- Address in
DAT_I : in std_logic_vector(15 downto 0); -- Data in
SEL_I : in std_logic_vector(7 downto 0);  -- Wishbone fluff
ACK_O : out std_logic; -- ACK for wishbone
DAT_O : out std_logic_vector(15 downto 0) -- Data out
);
end entity RAM;

architecture RTL of RAM is

subtype WORD is std_logic_vector(15 downto 0);
type MEMORY is array(0 to 2**ADDR_WIDTH-1) of WORD;
signal RAM256:MEMORY := (
"0100000000001011", -- 0: LOAD 11 (Bit 15-13 determines the instruction)
"0010000000001100", -- 1: ADD 12
"0110000000001101", -- 2: STO 13
-- ... Cut out to save space ... --
"0000000000000001", -- 11: 1
"0000000000000010", -- 12: 2
others => "0000000000000000");
begin

process(CLK_I, RST_I, ADR_I, STB_I)
    variable RAM_ADDR_IN : integer range 0 to 2**ADDR_WIDTH-1;
begin
if rising_edge(CLK_I) then
if (RST_I = '1') then
DAT_O <= (others => '0');
ACK_O <= '0';
else
RAM_ADDR_IN := conv_integer(ADR_I); -- Convert the adress vector to int
ACK_O <= '0';
if STB_I = '1' then
if (WE_I = '1') then
RAM256(RAM_ADDR_IN) <= DAT_I; -- Write what's on the DAT_I to memory at RAM_ADDR_IN
else
DAT_O <= RAM256(RAM_ADDR_IN); -- Read what's in the memory and put it on DAT_O
end if;
ACK_O <= '1';
end if;
end if;
end process;
end rtl;


[KaptenFrans]

*FIXED*
(Longer reply coming, but just to stop anyone from writing a huge post when the problem is already solved!!!)

[KaptenFrans]

What happens if you remove ADR_I, STB_I from the process's sensitivity list?

The same thing happened, I've tried changing everything! But thanks!

This may have something to do with the fact that you have an asynchronous reset.
  ....

Thank you very much for your reply! You were kind of on the right track! However you really helped me to improve my mental picture of what is going on, thank you once again!

You were probably right about Quartus optimizing away the constant WE_I. But I realised that the second process in my Control Unit(The output process) was only driven by a state change........  Just put my clk-signal in the process list and BAM, everything was back to normal. Too bad I wasted a whole days' work on it though..
It does however note that

Code: [Select]

Warning (276020): Inferred RAM node "RAM:b2v_RAM|RAM256_rtl_0" from synchronous design logic.  Pass-through logic has been added to match the read-during-write behavior of the original design.
Do you think this is something I need to worry about?

A related question, as I seem to have caught the attention of some pro's:
I get a critical warning:
"Critical Warning (332148): Timing requirements not met"
We have not even mentioned the TimeQuest analyzer during the course, so I guess that this is something I can ignore?

[xygor]

...Do you think this is something I need to worry about?

If you are reading and writing the same address the the same time, and you care about whether you get the old data or the new on the read then this matters.

A related question, as I seem to have caught the attention of some pro's:
I get a critical warning:
"Critical Warning (332148): Timing requirements not met"
We have not even mentioned the TimeQuest analyzer during the course, so I guess that this is something I can ignore?

You can ignore this if you are doing a behavioral simulation.  If you are doing a timing simulation or trying to run the design in an FPGA, then this matters.

[KaptenFrans]

If you are reading and writing the same address the the same time, and you care about whether you get the old data or the new on the read then this matters.

I have no plans for that, so ignore I shall.

You can ignore this if you are doing a behavioral simulation.  If you are doing a timing simulation or trying to run the design in an FPGA, then this matters.

I will run this on the DE2 dev-board eventually. Dammit. Thanks! =) 

[AndyC_772]

It's probably the asynchronous reset that's caused the timing failure. TimeQuest has no way to know what timing relationship, if any, exists between the reset and clock signals. It's bound to be the case that if they're truly asynchronous, sooner or later there will be a situation where both are changing very close together, and then the behaviour of your design is ambiguous.

It's a massive can of worms, basically, and another good reason to use a synchronous reset.

[KaptenFrans]

It's probably the asynchronous reset that's caused the timing failure. TimeQuest has no way to know what timing relationship, if any, exists between the reset and clock signals. It's bound to be the case that if they're truly asynchronous, sooner or later there will be a situation where both are changing very close together, and then the behaviour of your design is ambiguous.

It's a massive can of worms, basically, and another good reason to use a synchronous reset.

I see... That makes sense.
The reason I use an asynchronous reset is to adhere to "Rule 3.10" in the Wishbone specification:

All WISHBONE interfaces MUST be capable of reacting to [RST_I] at any time.

(Page 32: http://cdn.opencores.org/downloads/wbspec_b4.pdf)
It might be that I'm reading in more than it says(English is my second language, so some stuff goes over my head from time to time). I interpret that any Wishbone device should reset as soon the reset signal is driven high. But given what you wrote this seems quite odd and that it actually only means it should react to RST_I at any rising clock edge?
Especially when taking into consideration that "Suggestion 3.0"(Also on page 32) states that:

Some circuits require an asynchronous reset capability.

Which, to me at least, indicates that the default is for a device to be synchronous? That is also in line with the general design approach of HDL and what you wrote.
Am I correct?

[AndyC_772]

That same spec also says:

If an IP core or other SoC
component requires an asynchronous reset, then define it as a non-WISHBONE signal.
This prevents confusion with the WISHBONE reset [RST_I] signal that uses a purely
synchronous protocol

It's quite clear to me that the intent is for RST_I to be sampled, and acted upon, at the rising edge of CLK_I.

More generally, you can of course create designs in which a reset signal can be asserted at any time (ie. without being in any way synchronised to a clock), but you need to take precautions to avoid metastability. This is a whole topic in itself.

[KaptenFrans]

That same spec also says:

If an IP core or other SoC
component requires an asynchronous reset, then define it as a non-WISHBONE signal.
This prevents confusion with the WISHBONE reset [RST_I] signal that uses a purely
synchronous protocol

It's quite clear to me that the intent is for RST_I to be sampled, and acted upon, at the rising edge of CLK_I.

More generally, you can of course create designs in which a reset signal can be asserted at any time (ie. without being in any way synchronised to a clock), but you need to take precautions to avoid metastability. This is a whole topic in itself.

Thank you, once again. I'm sorry, I should have read more carefully. But I'm not yet used to reading specs of this magnitude and my work flow is quite flawed.

To anyone else finding this thread in the future:
I solved the time constraint warning by following this guide:
https://www.altera.com/en_US/pdfs/literature/ug/ug_tq_tutorial.pdf
It is written for Quartus9.1 but it was doable, at least in Quartus13.0sp1, with some slight alterations.