Is this code valid VHDL?

[Bruce Abbott]

I wanted to make a project for my Acorn Electron and I found this design for a 3 slot cartridge expansion. Because I don't need 3 slots, I decided to try modifying the CPLD source code to use a smaller chip. The original code compiled in Xilinx ISE V9.2i, but threw some warnings about unused pins and was not using the expected number of registers.

Here is the problem code:-

Code: [Select]

    sideways_select <= '1' when (A15 & A14 = "10") else '0';
What is supposed to happen is that signal 'sideways_select' gets set to '1' when (std_logic) inputs A15 and A14 have bit pattern '10', ie. when A15 is high and A14 is low. However in reality it apparently always equates to '0', so any logic that is dependent on sideways_select being 1 gets optimized out. 

I fixed the problem by changing it to this:-

Code: [Select]

    sideways_select <= '1' when (A15 = '1') and (A14 = '0') else '0';
As a beginner to VHDL I often come across stuff that doesn't make much sense to me, and according to the designer his code 'works really well!', so... it is actually valid code, and should it do what was intended? Or if not, why doesn't ISE flag it?



 

     

[Bassman59]

I wanted to make a project for my Acorn Electron and I found this design for a 3 slot cartridge expansion. Because I don't need 3 slots, I decided to try modifying the CPLD source code to use a smaller chip. The original code compiled in Xilinx ISE V9.2i, but threw some warnings about unused pins and was not using the expected number of registers.

Here is the problem code:-

Code: [Select]

    sideways_select <= '1' when (A15 & A14 = "10") else '0';
What is supposed to happen is that signal 'sideways_select' gets set to '1' when (std_logic) inputs A15 and A14 have bit pattern '10', ie. when A15 is high and A14 is low. However in reality it apparently always equates to '0', so any logic that is dependent on sideways_select being 1 gets optimized out. 

I fixed the problem by changing it to this:-

Code: [Select]

    sideways_select <= '1' when (A15 = '1') and (A14 = '0') else '0';
As a beginner to VHDL I often come across stuff that doesn't make much sense to me, and according to the designer his code 'works really well!', so... it is actually valid code, and should it do what was intended? Or if not, why doesn't ISE flag it?

That's an ancient version of ISE!

Anyways, I am surprised that the synthesizer didn't complain about your code. I assume that A15, A14 and sideways_select are all std_logic types. The code needs a typecast because there's nothing telling the analyzer what you should get when you concatenate two std_logic signals.

Correct code would be:

Code: [Select]

sideways_select <= '1' when std_logic_vector'(A15 & A14) = "10" else '0';
The typecast is std_logic_vector'( ....) and note the presence of the apostrophe.

[Bruce Abbott]

That's an ancient version of ISE!

An ancient version for an ancient guy with an ancient computer that doesn't like bloatware. 9.2i only takes up a mere 3GB on my hard drive! And since I only intend to use ancient CPLDs it's perfectly adequate... 

I am surprised that the synthesizer didn't complain about your code.

It's not my code, which why I too was surprised. It's supposed to come from a fully working project.

Here are the warning messages:-

Code: [Select]

=========================================================================
*                       Advanced HDL Synthesis                          *
=========================================================================

WARNING:Xst:2677 - Node <2> of sequential type is unconnected in block <bank>.
WARNING:Xst:2677 - Node <3> of sequential type is unconnected in block <bank>.
WARNING:Xst:2677 - Node <1> of sequential type is unconnected in block <bank>.
...
=========================================================================
*                         Low Level Synthesis                           *
=========================================================================
WARNING:Xst:2677 - Node <bank_1> of sequential type is unconnected in block <minus_one>.
WARNING:Xst:2677 - Node <bank_3> of sequential type is unconnected in block <minus_one>.
WARNING:Xst:2677 - Node <bank_2> of sequential type is unconnected in block <minus_one>.
...
Started : "Fit".
WARNING:Cpld:1007 - Removing unused input(s) 'D<1>'.  The input(s) are unused
   after optimization. Please verify functionality via simulation.
WARNING:Cpld:1007 - Removing unused input(s) 'D<2>'.  The input(s) are unused
   after optimization. Please verify functionality via simulation.
WARNING:Cpld:1007 - Removing unused input(s) 'D<3>'.  The input(s) are unused
   after optimization. Please verify functionality via simulation.These all occur because there is a 4 bit register (called 'bank')which is referenced only when sideways_select is 1. For some reason D0 is allocated, resulting a 1 bit register being synthesized. It took me a while to figure out that the reason had nothing to do with the register itself.
 

I assume that A15, A14 and sideways_select are all std_logic types.

Yes, that is correct.

The code needs a typecast because there's nothing telling the analyzer what you should get when you concatenate two std_logic signals.

Correct code would be:

Code: [Select]

sideways_select <= '1' when std_logic_vector'(A15 & A14) = "10" else '0';
The typecast is std_logic_vector'( ....) and note the presence of the apostrophe.

Thanks for that, now I know how and why to use a typecast! In this case it's only a single combination of two bits so it's probably just as easy to treat them separately, but could definitely be useful when larger numbers are involved.   

The original code also had this in it:-

Code: [Select]

    signal A_high : std_logic_vector(7 downto 0);
    ...
    A_high <= A15 & A14 & A13 & A12 & A11 & A10 & A9 & A8;
So I did this:-

Code: [Select]

    sideways_select <= '1' when A_high(7 downto 6) = "10" else '0';
Which solution would be better;  typecast the port inputs, or use the internal std_logic_vector signal - or does it not matter? They both used identical resources so I guess any differences were optimized out.

Anyway I will go with your typecast because it is closest to the original intent.

 
   

[legacy]

ISE-{v10.1 .. v14.7} has similar problems with functions!
A trick ... you can use a signal to describe A15-A14 as an unique entity!

Code: [Select]

signal CS_A1514 : std_logic_vector(1 downto 0):= (others => '0'); -- default

...

CS_A1514 <= A15 & A14;

sideways_select <= '1' when CS_A1514 = "10" else '0';


anyway, I prefer using explicit "when-cases" inside a Process() to describe the logic! And this fixes the problem for me.

Code: [Select]

  process
  (
    CS_A1514
  )
  begin
            case CS_A1514 is
              when b"10"  => sideways_select <= '1';
              when others => sideways_select <= '0';
            end case;
  end process; 


p.s.
when I said that I saw a lot of hipster code on OpenCores: I meant something similar, and I wouldn't be surprised if code works on Altera but not on Xilinx!

When someone claims "it works well here!", it usually does mean nothing 

[legacy]

Code: [Select]

    signal A_high : std_logic_vector(7 downto 0);
    ...
    A_high <= A15 & A14 & A13 & A12 & A11 & A10 & A9 & A8;

Code: [Select]

    sideways_select <= '1' when A_high(7 downto 6) = "10" else '0';


this looks good, as well!

[NorthGuy]

Here is the problem code:-

Code: [Select]

    sideways_select <= '1' when (A15 & A14 = "10") else '0';

I would just use:

Code: [Select]

sideways_select <= A15 and not A14;

[Bassman59]

I would just use:

Code: [Select]

sideways_select <= A15 and not A14;

That won't work with the ancient VHDL'87 supported by ISE 9.1.

[Bassman59]

It's not my code, which why I too was surprised. It's supposed to come from a fully working project.

Here are the warning messages:-

Code: [Select]

=========================================================================
*                       Advanced HDL Synthesis                          *
=========================================================================

WARNING:Xst:2677 - Node <2> of sequential type is unconnected in block <bank>.
WARNING:Xst:2677 - Node <3> of sequential type is unconnected in block <bank>.
WARNING:Xst:2677 - Node <1> of sequential type is unconnected in block <bank>.
...
=========================================================================
*                         Low Level Synthesis                           *
=========================================================================
WARNING:Xst:2677 - Node <bank_1> of sequential type is unconnected in block <minus_one>.
WARNING:Xst:2677 - Node <bank_3> of sequential type is unconnected in block <minus_one>.
WARNING:Xst:2677 - Node <bank_2> of sequential type is unconnected in block <minus_one>.
...
Started : "Fit".
WARNING:Cpld:1007 - Removing unused input(s) 'D<1>'.  The input(s) are unused
   after optimization. Please verify functionality via simulation.
WARNING:Cpld:1007 - Removing unused input(s) 'D<2>'.  The input(s) are unused
   after optimization. Please verify functionality via simulation.
WARNING:Cpld:1007 - Removing unused input(s) 'D<3>'.  The input(s) are unused
   after optimization. Please verify functionality via simulation.These all occur because there is a 4 bit register (called 'bank')which is referenced only when sideways_select is 1. For some reason D0 is allocated, resulting a 1 bit register being synthesized. It took me a while to figure out that the reason had nothing to do with the register itself.

Without seeing the rest of the code, I can't guess as to why this optimization occurred.

The original code also had this in it:-

Code: [Select]

    signal A_high : std_logic_vector(7 downto 0);
    ...
    A_high <= A15 & A14 & A13 & A12 & A11 & A10 & A9 & A8;
So I did this:-

Code: [Select]

    sideways_select <= '1' when A_high(7 downto 6) = "10" else '0';

The original code writer might have created the A_high vector because he didn't know about typecasts. Or maybe there was a reason why the input pins were called A15, A14 etc. instead of A(15 down to 0). But that concatenation is a good trick.

Which solution would be better;  typecast the port inputs, or use the internal std_logic_vector signal - or does it not matter? They both used identical resources so I guess any differences were optimized out.

It doesn't matter. I would have named the input puts A(15 downto 0) and been done with it.

[Bruce Abbott]

I would just use:

Code: [Select]

sideways_select <= A15 and not A14;

That won't work with the ancient VHDL'87 supported by ISE 9.1.

Actually it works fine, even in the positively prehistoric ISE Webpack 4.2! This version is so old that it can't handle spaces in filenames. It doesn't do fancy formatted reports and stuff, but has one big advantage - on my machine it's about 10 times faster! This is important to a coder like me who is constantly making silly mistakes (on 9.2i I spent half my time watching that little blue diamond spin around...).

Fast, efficient, no mucking around and very little bloat (only a miserable 300MB on the hard drive) - just how I like it. 
 
Here 's the logic equations resulting from the code above:-

Code: [Select]

/cart0_noe  =  /a14 * a15 * /"bank<1>" * /"bank<2>" * /"bank<3>"   

/cart2_noe  =  /a14 * a15 * "bank<1>" * /"bank<2>" * /"bank<3>"   

/cart4_noe  =  /a14 * a15 * /"bank<1>" * "bank<2>" * /"bank<3>"   

/cart_noe2  =  /a14 * a15 * cart_romqa * /"bank<1>" * "bank<2>" * "bank<3>"   
 
We can see that  'A15 and not A14' has indeed been correctly combined with the bank select register bits to create the cartridge enable outputs. 

Interestingly, (unlike 9.2i) when presented with the original bad code, ISE 4.2 barfed and directed me to the line with the actual error.  Here's the error message:-

Code: [Select]

ERROR:HDLParsers:810 - C:/code/CPLD/webpack4/minus0/minus0.vhd Line 64. = has two possible definitions in this scope.
Another plus for version 4.2!

[legacy]

Why don't you use ModelSim (StudentEdition) to check the VHDL, and then ISE to see if it fits the RTL-model?

[legacy]

Or Simili/Sonata! They still offer (by email) a free license for their tool!

[Bruce Abbott]

Why don't you use ModelSim (StudentEdition) to check the VHDL, and then ISE to see if it fits the RTL-model?

I could, but first I need to know what the chip is supposed to do! (remember that this is not my design...). I was up until 2am this morning reading about  how the Acorn Electron selects its sideways ROMs, so hopefully I understand it now.

And to be honest, I don't think the time spent simulating such simple logic is worth the effort. I am now concentrating on creating a PCB for the project, and once this is done I will be able to test the actual hardware.