VHDL / Interfaces

Interface definitions for VHDL-2019.
Other
12 stars 2 forks source link

Naming Methodology #10

Open JimLewis opened 4 years ago

JimLewis commented 4 years ago

Hi Patrick, I see lots of code. However, I think we might want to agree on naming methodology as it would be nice if all were the same.

I would like something that is obvious - even to people who don't have the secret decoder ring for the naming convention. The conventions that use T_ or _T are a reflection of a generation of people who could not type.

Going further mode views are new. When people see V_ they are going to thing variable.

Best Regards, Jim

Paebbels commented 4 years ago

The proposal is using

Unfortunately, VHDL is not very clever when it comes to names. Everything resides in a single scope, therefore we need to distinguish types from objects and views.

JimLewis commented 4 years ago

I propose that instead of

Paebbels commented 4 years ago

@JimLewis please review #12 and accept if you agree.

LarsAsplund commented 4 years ago

@Paebbels @JimLewis I think simplicity is important considering that there are many people that will have to adapt to a style they are not used to. Looking at the code we have identifiers like DifferentialLane_Interface, DifferentialLane_TransmitterView and DifferentialLane_Interface_Vector. Where to use camel case and where to use underscore becomes complex. Is there a significant value in that complexity? Why not just underscores or just camel case?

JimLewis commented 4 years ago

@LarsAsplund I understand your concern. Here the '_' provides visual separation of the name vs. the modifier(s). DifferentialLate is the base name. _Interface designates that it is an interface. _TransmitterView designates that it is a view for the transmitter. _Interface_Vector designates that it is an array of interfaces.

OTOH, if we were to write, Differential_Lane_Transmitter_View then one does not readily see the name DiferentialLane from the modifiers. One does not see that it is a single modifier TransmitterView vs multiple separate modifiers like _Interface_Vector.

Likewise the same could be said if we were to write it as, DifferentialLaneTransmitterView.

JimLewis commented 4 years ago

@Paebbels
Looks like all of the files have tabs in them. The tabs render in GitHub as 8 spaces.

Can we propose to not use tabs and have the editors convert tabs on entry to spaces instead? This is particularly important when copying entities and making them into components as suddenly the tab stops are not aligned anymore. In addition to make a component instance, I like to copy the component or entity and edit from there. It really helps with column cuts to have only spaces as the alignment then never shifts.

JimLewis commented 4 years ago

I propose that for packages, we use some sort of naming that differentiates their names,

Can we alias design unit names - such as packages to have a new name? This would help transition from an older naming style to a newer one. If not, we need to add it as part of the language revision.

LarsAsplund commented 4 years ago

@JimLewis I understand the idea but is the value of being clear about the modifiers really worth the extra complexity? If there is a significance in terms of understanding what is meant it would mean that the identifier cannot be understood if you read it for me since they all sound alike. Doesn't that suggest that there is a problem with the fundamental naming?

I think the fact that you're suggesting that it should be _Interface but not _Pkg is a good example of the type of confusion complex rules leads to ;-)

LarsAsplund commented 4 years ago

Considering that interface is part of the base name for AXI, SPI and others I think it should be that for the differential interface as well. The record should have a type suffix like any other type identifier.

Paebbels commented 4 years ago

@JimLewis using

is intended and correct.

It's problem of GitHub not applying the rules of .editorconfig. The repository has a matching editor configuration files that is nowadays understood by almost all editors either directly or via plugin.

See: https://github.com/VHDL/Interfaces/blob/dev/.editorconfig#L3-L10

[*]
charset = utf-8
# end_of_line = lf
insert_final_newline = true
trim_trailing_whitespace = true
indent_style = tab
indent_size = 2
tab_width = 2
JimLewis commented 4 years ago

@Paebbels And then when if you print - even to pdf it is very bad.

You always have tools that do not understand your edit rules.

JimLewis commented 4 years ago

File naming? Can I propose that we have the file name match the design unit name. IE: no - characters.

For the file extension, are we doing .vhd or .vhdl?

LarsAsplund commented 4 years ago

Another simplification is to not treat abbreviations differently. Then there is no discussion about why it's AXI and not PKG.

Paebbels commented 4 years ago

To answer some of the questions above:

Modifiers

Yes, I agree with Jim, we need a more clear naming and readability then a SingleLongCamelCaseWord or a chained_long_snake_case_word.

This collection of interfaces will grow and get lots of interface variations. Of cause we will try the best to minimize it by using partially constrained/unconstrained types, but we'll have variation. For each interface we'll then have even more views. So we need a naming scheme that can handle it. The naming scheme must also allo users to create names in a similar style, ever if this repository is not providing these names.

Package names

Package names are not in the same namespace as types and views. I see no need to apply such a suffix to design units too.

File extension

Abbreviations

There are two possible styles for abbreviations:

Dashes in filenames

The official name is AXI4-Lite. Hence the file name in my proposed upload.

Suggestions: We can remove dashes and merge words.

LarsAsplund commented 4 years ago

@Paebbels Using tabs for indentation and space for alignment is also something I consider more complex than necessary. As you know I've done some statistical analysis of GitHub and a majority, about 75% if I recall correctly, use spaces instead of tabs and a majority uses 2 spaces for every indentation level. For VHDL files that is. Regardless of the style chosen the majority of users will have to adapt to some extent. We want to keep that to a minimum, that is go with the majority. There can of course be cases where the majority is wrong about something which has significant value. The value in naming conventions is primarily readability and the only study I read on indentation (let's see if I can find it) concludes that there is no difference in readability if your indentation steps are in the 2 - 4 range but it's worse outside of that range. Interestingly people in the study felt that 5 was good although the data showed it was not. The idea with tabs is that people can set it to whatever they prefer. The study suggests that that preference may be factually wrong. Any step in the 2 - 4 range should be equally readable and if the majority uses 2 I think we should fix it to that by using spaces.

LarsAsplund commented 4 years ago

@Paebbels

LarsAsplund commented 4 years ago

I think I found the indentation study: https://www.cs.umd.edu/~ben/papers/Miara1983Program.pdf However, a later study trying to replicate the first but with modern eye-tracking devices (https://www.infosun.fim.uni-passau.de/publications/docs/Bauer19.pdf) didn't reach the same conclusion. Instead they concluded that the indentation depth has no significance which suggest that we can use anything as long as we're consistent (or we will have a lot of file diffs only related to spacing). If it doesn't matter I would still suggest taking the route of least resistance, that is use what most people use. 2 spaces.

tmeissner commented 4 years ago

I have to ask what the intention of these interfaces is. Is it for testing the VHDL-19 features in the various simulators / tools? Or should they serve as examples for using interfaces in RTL designs for all users of VHDL-19?

Honestly, with these naming conventions and other style enforcements, I wouldn't never use these interfaces as they are in projects in our company. But this would be almost the same with other rules, so maybe that doesn't matter.

LarsAsplund commented 4 years ago

@tmeissner The intention is that you would use them in your company code rather than write them yourself. Just like you would use types in ieee, vunit_lib, OSVVM and other external libs which may or may not share your company style. If the style presented here is a show stopper for that then tell us more.

tmeissner commented 4 years ago

No show stopper, but sometimes difficult. We use suffixes and prefixes for example. An excerpt of our style:

We also use code with other styles, no real problem.

I heard the first time of .editorconfig, and I assume that only few people know that feature either. We use sublime text and have user defined configs in that editor. There are even people which use the editors of the vendor tools and I don't know if you can use that settings in these editors either ...

eine commented 4 years ago

Regarding vhdl vs vhd, see: https://ghdl.readthedocs.io/en/latest/quick_start/hello/README.html?highlight=extension#hello-world-program

Both .vhdl and .vhd extensions are used for VHDL source files, while .v is used for Verilog.

Since, extension .vhd is also interpreted as a Virtual Hard Disk file format, some users prefer .vhdl, to avoid ambiguity. This is the case with GHDL’s codebase. However, in order to maintain backward-compatibility with legacy DOS systems, other users prefer .vhd.

LarsAsplund commented 4 years ago

@tmeissner The more rules the more painful to adapt but even with few rules most people would probably have to adapt to some extent. I like the thinking of new languages where the coding style is part of the LRM. Kill the discussion from day one. The VHDL standard is a mix of all possible coding styles so I think that ship has sailed. I think the best we can do from the standard side is to make type casting between types where only naming differs very simple. @Paebbels Didn't you just started a discussion on that?

The proposed interfaces are really just a bunch of named wires so if most people were to rename those wires to match their own style or use some clever type casting mechanism then much of the value in this repo is lost.

eine commented 4 years ago

I think the best we can do from the standard side is to make type casting between types where only naming differs very simple. @Paebbels Didn't you just started a discussion on that?

http://grouper.ieee.org/groups/1076/email/msg01278.html

The proposed interfaces are really just a bunch of named wires so if most people were to rename those wires to match their own style or use some clever type casting mechanism then much of the value in this repo is lost.

I believe that many people are going to rename those wires, and it is still ok. I think that the main purpose of this repository is to showcase Interfaces, which is an unknown feature yet, but which is a game changer for the traditional "VHDL is so verbose" mantra. I'm quite sure that we all want tools to support interfaces ASAP.

Apart from that, it serves as a common definition for user A and user B to know how their customized interfaces match each other. This is quite stupid for simple interfaces, but it can be complex with composed interfaces, such as AXI. Following Patrick and Jim's discussion about matching/aliasing/spaceshipping types, if each user maps/binds custom interfaces to the common definitions here, any number of different styles can be mixed, without mapping each pair explicitly.

LarsAsplund commented 4 years ago

I think that the main purpose of this repository is to showcase Interfaces, which is an unknown feature yet, but which is a game changer for the traditional "VHDL is so verbose" mantra. I'm quite sure that we all want tools to support interfaces ASAP.

If that's the purpose we should rethink what we're doing and develop just a few examples

Apart from that, it serves as a common definition for user A and user B to know how their customized interfaces match each other. This is quite stupid for simple interfaces, but it can be complex with composed interfaces, such as AXI. Following Patrick and Jim's discussion about matching/aliasing/spaceshipping types, if each user maps/binds custom interfaces to the common definitions here, any number of different styles can be mixed, without mapping each pair explicitly.

Matching/aliasing/spaceshipping would save the day. Could it be that there is no point in publishing all these interfaces before we have this in place?

That brings me to another question. What are the limits on how quickly a new LRM can be released? Are there limits on how small the updates can be. Basically, what is the agility of IEEE if we push it to its extreme?

LarsAsplund commented 4 years ago

This is roughly how I think about code style development

code style

Typical problems are:

JimLewis commented 4 years ago

That brings me to another question. What are the limits on how quickly a new LRM can be released? Are there limits on how small the updates can be. Basically, what is the agility of IEEE if we push it to its extreme?

You have to get a PAR. DASC has to approve the PAR (2 weeks). Then NESCOM needs to approve the PAR. NESCOM meets the quarterly IEEE SA meetings. PARS must be submitted well before that date (I think 30 days. When the standards revision work is done we solicit for the the ballot pool - which is 30 to 45 days. Ballot pool cannot be open for more than a maximum time or it automatically dissolves. Then ballot, which I think is another 30 days. Potential reballoting because someone did not like your terminology. Submit for RevCom approval which synchronizes with the IEEE SA quarterly meetings and must be submitted well in advance of the date.

NESCOM and REVCOM reviews start before the meeting date and the meeting is just a formal approval. They also let you correct small things as part of the process. So I look at it as a coaching session to make sure things are right - or are right by the time the process is done.

This time when I asked about getting a PAR, DASC asked what we wanted to work on. So for each revision we would need to justify what we are doing.

JimLewis commented 4 years ago

@LarsAsplund

I think the best we can do from the standard side is to make type casting between types where only naming differs very simple. @Paebbels Didn't you just started a discussion on that?

Deja. Already done in VHDL-2019. See: http://www.eda-twiki.org/cgi-bin/view.cgi/P1076/LCS2016_075

JimLewis commented 4 years ago

@eine @LarsAsplund That said, to do the VHDL-2019 supported type conversion of records, the order of the elements in the records must be identical. There is no mapping between one and the other. Hence, if someone does not want to use the same naming style, if they use this as a template and don't change the order, then they can use type conversion (aka type casting).

eine commented 4 years ago

If that's the purpose we should rethink what we're doing and develop just a few examples

Apart from that, whatever exists in this repo will be really valuable for new users (those who don't have other styles they need to match/reuse) to use as-is. This applies to examples of OSVVM or VUnit verification components which use these definitions. Although those are not part of this repo per se, they are part of the showcase. This builds on your concerns in the reflector about what should be LRM, what IEEE libs and what community libs.

@JimLewis what about mapping, for example, the following?

    type SimpleVGA_Interface is record
        Red            : unresolved_unsigned;
        Green          : unresolved_unsigned;
        Blue           : unresolved_unsigned;
        HorizontalSync : std_ulogic;
        VerticalSync   : std_ulogic;
    end record;
    type VGARGB_Interface is record
        RGB            : std_ulogic_vector(2 downto 0);
        HorizontalSync : std_ulogic;
        VerticalSync   : std_ulogic;
    end record;
JimLewis commented 4 years ago

@eine The language cannot help you there - even if Red, Green, and Blue are only 1 bit each (IE match in size of the RGB element). We would need something else to help here.

JimLewis commented 4 years ago

@eine I added your example (slightly modified) to http://www.eda-twiki.org/cgi-bin/view.cgi/P1076/MapFunctions

Feel free to add to it and/or take it over. There may be other stuff from the previous revision.

Paebbels commented 4 years ago

The longer this discussion gets about naming style as part of coding style, we also have #27. We need so kind of easy mapping or aliasing of simple and complex wire structures in record structures.

With this repository, I would like to establish a common sense interface description as de facto standards. At best we can add official second and third names like we have with I²C, PMBus, SMBus and TWI (two wire interface). We also should correct misleading naming confusions/mistakes in standardized interfaces.

With such a mapping / aliasing, we can offer alternatives and moreover users can change an interface to there style and feelings.

JimLewis commented 4 years ago

Some of Lars comments about mixing _ with camel case are sinking in. If we do the suffixes right, I don't think we need the _. Ie: if we define major names - like View and then allow View to have a modifier - in, out, Super, Minion

We need some more complete examples of naming in use.

I am concerned with Interface. What do we name the signal that connects to the interface? Lets say we have UartInterface, then what is the signal named. For AXI, I named my AxiBus, but for Uart that does not work. Hence that leads me to thinking we need a UartInterfaceType (the type) and UartInterface (the signal).

LarsAsplund commented 4 years ago

I am concerned with Interface. What do we name the signal that connects to the interface? Lets say we have UartInterface, then what is the signal named.

In my own code I called the record the interface type (foo_if_t) and the signal is the interface (foo_if). It follows the same logic I use for state types (foo_state_t) and the signal/variable holding the state (foo_state).

I would also suggest that we define the closely related transaction type that can be used to represent transactions at a higher abstraction layer. For example, the AXI stream interface type is a record containing an AXI stream transaction together with ready and valid

JimLewis commented 4 years ago

How close do we want to match the name that is part of a standards (or defacto standards) document?

I bring this up as I see the AXI abbreviation of Addr being lengthened to Address, Strb to Strobe, and Prot to Protect. Whether it is Strobe or Strb, or Prot or Protect, one still has to look them up in the standard document to determine what they actually mean.

If this is not going to be part of a standards effort, I think if we deviate too far from the established names, like them or not, people will not use this.