HDL Code formatting.

[hamster_nz]

Do you format your HDL source in a way that would be unusual for program code source, to hint at the underlying structure you are aiming to implement?

What does your local "coding culture" find acceptable?

(Not looking to fight any Python coders, but if the gloves come off....  )

As an example this bit of code:

Code: [Select]

...
            case add_extra_word is
                when aew_add_08 =>    d_out_ctrl <= "1110"; d_out <= x"000000" & crc_08(31 downto 24);
                when aew_add_16 =>    d_out_ctrl <= "1100"; d_out <= x"0000" & crc_16(31 downto 16);
                when aew_add_24 =>    d_out_ctrl <= "1000"; d_out <= x"00" & crc_24(31 downto 8);
                when aew_idle   =>
                    case d_ctrl is
                       when "1110" => d_out_ctrl <= "0000"; d_out <= (not s08(23 downto 0)) & d( 7 downto 0);
                       when "1100" => d_out_ctrl <= "0000"; d_out <= (not s16(15 downto 0)) & d(15 downto 0);
                       when "1000" => d_out_ctrl <= "0000"; d_out <= (not s24( 7 downto 0)) & d(23 downto 0);
                       when "1111" => if in_packet = '1' then
                                           d_out_ctrl <= "0000"; d_out <= crc_32;
                                      end if;
                       when others => 
                    end case;
            end case;
...

But in a programming language it would be

Code: [Select]

...
            case add_extra_word is
                when aew_add_08 =>
                    d_out_ctrl <= "1110";
                    d_out <= x"000000" & crc_08(31 downto 24);
                when aew_add_16 =>
                    d_out_ctrl <= "1100";
                    d_out <= x"0000" & crc_16(31 downto 16);
                when aew_add_24 =>
                    d_out_ctrl <= "1000";
                    d_out <= x"00" & crc_24(31 downto 8);
                when aew_idle   =>
                    case d_ctrl is
                       when "1110" =>
                            d_out_ctrl <= "0000";
                            d_out <= (not s08(23 downto 0)) & d( 7 downto 0);
                       when "1100" =>
                            d_out_ctrl <= "0000";
                            d_out <= (not s16(15 downto 0)) & d(15 downto 0);
                       when "1000" =>
                            d_out_ctrl <= "0000";
                            d_out <= (not s24( 7 downto 0)) & d(23 downto 0);
                       when "1111" =>
                            if in_packet = '1' then
                                d_out_ctrl <= "0000";
                                d_out <= crc_32;
                            end if;
                       when others => 
                    end case;
            end case;
...


[tggzzz]

HDLs are programming languages. They are based on event-driven dataflow concepts rather than sequential procedural concepts. Those that master them are time lords, masters of time and space

Formatting is only a small part of clarity; there are more significant contributors. A common formatting standard is more important than the detailed specification of a standard.

You can write Fortran in HDLs too

[SiliconWizard]

And to add to this, I must say I use both styles shown above, be it for HDLs or software programming, depending on a few factors. The main one is the overall length of the statement after the when clause (or after a case in C, ...). If it's too long, I'll break it in several lines, otherwise I'll put it on the same line. Another point beyond the length is that I avoid using the "same-line" style if there are more than 1 statement after the when clause, because then I find it less readable (a corollary point with this is that if there's more than 1 statement, then it's not unlikely that there will be even more in the future after modifications/improvements, and then, you'll end up having to reformat it with several lines anyway.)

[free_electron]

i use the first style a lot. make sure the binary masks line up vertically. This gives an immediate visual overview of what is being applied and when. if it needs alteration it is very easy to find visually without actually having to 'read' ever line of code.
for visual structure i would even line up the & operators before the crc_...

[Berni]

i use the first style a lot. make sure the binary masks line up vertically. This gives an immediate visual overview of what is being applied and when. if it needs alteration it is very easy to find visually without actually having to 'read' ever line of code.
for visual structure i would even line up the & operators before the crc_...

Yep i do it similarly too.

I find that HDL tends to be pretty "copy-pastey" where similar lines of code repeat with slight differences. So in these cases i find it convenient to arrange it in such a way that it forms a nice clean grid(including adding extra whitespace to line things up). That way i can look down the columns and clearly see the slight differences.

In regular programing languages i don't do it as much, but still do it for things that kind of look like a table. Like for example a large block of #define lines that list out the registers of a chip would be extra indented to nicely line up.

[tom66]

My HDL and my C++ have remarkably similar syntax:

- Units are broken into modular components (i.e. there's a FIFO controller, there's a datamover block, there's an AXI controller...)  Some of these components are quite small (20-50 lines) and are integrated into the same module file; others are split into their own file and have a testbench associated with them once they exceed a certain size.

- I line things up where it makes it clearer what the intention of the engineer was.

- There are liberal comments throughout.

I'm definitely more in 'style 2' camp than 'style 1'.

[Bassman59]

I never (rarely ever) use your first form. Always the second.

[DiTBho]

it's parametric, it depends on the screen size 

[SiliconWizard]

I would also add that I almost never need to align literals in this way because I favor defining constants and using them instead. More readable and more maintainable, while not requiring the urge to align things. I avoid using literals in HDLs just like I avoid using them in programming languages in general. "Hard-coded values" are messy. Some people using HDLs may find them "easier" because that looks "closer to the metal", but that's a wrong approach IMHO.

So for instance,

Code: [Select]

d_out_ctrl <= "1110";Isuppose this is some control signals. What does "1110" mean here (unless you navigate the whole source code, possibly in other files, to figure it out)? Just define constants, give that a name that conveys a meaning, and instantly you know what it does. And, you won't need to align anything.

[Berni]

Having "magic bits" like that is definitely a bad idea.

For aligning things i meant the more boilerplate-ish parts of code that inevitably becomes a long drawn out repeating sequence because it is mapping a bunch of things thing onto another bunch of things.

For example when you build a memory mapped peripheral in a FPGA. You often need to map the bits in a control register to actual peripheral control signals. Then you also need to group together a bunch of status bits into a status register. You might also process the read/write operations in a big switch case statement where a few registers are a special case (like autoincrement fifo read) but most other cases are simply mapping a register to a memory address.

[hamster_nz]

Having "magic bits" like that is definitely a bad idea.

For aligning things i meant the more boilerplate-ish parts of code that inevitably becomes a long drawn out repeating sequence because it is mapping a bunch of things thing onto another bunch of things.

For example when you build a memory mapped peripheral in a FPGA. You often need to map the bits in a control register to actual peripheral control signals. Then you also need to group together a bunch of status bits into a status register. You might also process the read/write operations in a big switch case statement where a few registers are a special case (like autoincrement fifo read) but most other cases are simply mapping a register to a memory address.

Those bits aren't that magic... they are the 4-bit 'byte enables' for the 32-bit data values.

The context isn't that clear because of it being a code snippet, but having BYTE_ENABLE_0000 and BYTE_ENABLE_1111 isn't really that helpful... They are labelled "ctrl" because the 1 signifies "control symbols" vs "data symbols" in the protocol.

[laugensalm]

I'd say: Both variants are readable and 'nice'. I'm a pragmatic VIteran (pun indented :-)), a `YP` key sequence just performs faster copying of lines than having to fiddle with indentation and block mode. Since your example is 'compact' for both notations, it's hard to decide for one under the copy&paste criteria, as long as the character of a lookup table is clearly visible.
However when more complexity and if..else constructs are introduced, I'd go for notation (2) -- under the future maintenance aspect.
Other than that I think it's a good rule to follow the max. 80 chars line width.
If VHDL wasn't so loaded with typing overhead, I'd go with a LUT (static) or parallel MUX notation (dynamic) in this case (this is where Python HDLs allow way more fun).
My experience in all these years of HDL experiments: less copy/paste coding, less text to identify, less errors.

[SiliconWizard]

Those bits aren't that magic... they are the 4-bit 'byte enables' for the 32-bit data values.

Then, possibly renaming the 'd_out_ctrl ' signal to something including 'byte enable' would make the code easier to figure out. =)

The context isn't that clear because of it being a code snippet, but having BYTE_ENABLE_0000 and BYTE_ENABLE_1111 isn't really that helpful... They are labelled "ctrl" because the 1 signifies "control symbols" vs "data symbols" in the protocol.

While excessively using constants to absurd levels is not helpful at all, your interpretation of writing constants here is pretty odd. To put it lightly.
Would be a bit like, say, having a constant that is supposed to be the max value of some variable, that would be say 100, and defining the constant as "VALUE_100 = 100" instead of "VALUE_MAX = 100", or something like that. If you get the idea.

Since those are byte enables, here, if I wanted to define constants, I would likely name them BYTE_ENABLE_8, BYTE_ENABLE_16, etc... instead. Makes more sense. IMHO.

They are labelled "ctrl" because the 1 signifies "control symbols" vs "data symbols" in the protocol.

Yeah, to each their choice of signal/variable naming. This is more an art than a science, and relatively subjective. To relate to what I just wrote above. But if the signal doesn't control anything else than byte enables, then I'd personally stick to my view.

[hamster_nz]

Yeah, to each their choice of signal/variable naming. This is more an art than a science, and relatively subjective. To relate to what I just wrote above. But if the signal doesn't control anything else than byte enables, then I'd personally stick to my view.

Fair enough not trying to sway you either way. It's an assumption on my part that somebody working on this code will be familiar with IEEE 802.3ae XGMII, and I'm following the naming used in the spec and documentation.

[miken]

I usually like wordy descriptive naming but recently I've been writing a 16550 (well, 16450 so far) and I decided to mostly use the mnemonics from the datasheet. So if there are established names I figure might as well use 'em.

As for local coding culture, have you guys seen this style in the wild...

Code: [Select]

module foo (
input clk
,input rst
,input bar
,output baz
);
I can kind of understand why, having made many port-comma omissions myself, but I find it rather painful aesthetically.

[SiliconWizard]

Yeah, to each their choice of signal/variable naming. This is more an art than a science, and relatively subjective. To relate to what I just wrote above. But if the signal doesn't control anything else than byte enables, then I'd personally stick to my view.

Fair enough not trying to sway you either way. It's an assumption on my part that somebody working on this code will be familiar with IEEE 802.3ae XGMII, and I'm following the naming used in the spec and documentation.

Thanks for the explanation. So, if you're "complying" to some standard, that's fair enough. (And no, I'm not familiar with XGMII.)
In which case, since the signal name doesn't hold much details, using constants for byte enables would certainly help readability. But as I said, this has a subjective part.

Using constants, IMO, is justified in the following cases:
- When they help readability,
- When the values are subject to change,
- When the values are reused in several places in the code (the most important point++).

[free_electron]

I usually like wordy descriptive naming but recently I've been writing a 16550 (well, 16450 so far) and I decided to mostly use the mnemonics from the datasheet. So if there are established names I figure might as well use 'em.

As for local coding culture, have you guys seen this style in the wild...

Code: [Select]

module foo (
input clk
,input rst
,input bar
,output baz
);I can kind of understand why, having made many port-comma omissions myself, but I find it rather painful aesthetically.

i use this :

Code: [Select]

module foo (
  input clk,rst,bar
  output baz
);