-
Notifications
You must be signed in to change notification settings - Fork 25
Coding Style
The primary guideline is to follow the CMS guidelines, as outlined in this CMSSW style guide. The code submitted in a pull request will automatically be subjected to the same clang-tidy checks that CMSSW code is, and code will have to pass those checks before being accepted into the master branch.
In this document let's try to collect our best practices.
- Avoid hard-wired constants buried in code.
- Commonly varied constants should be declared in Constants.h type-file or via CMSSW cfg params.
- Class data members end with underscore. e.g.
raw_. - Use setters and getters to hide implementation;
- Follow CMSSW standards such that the setter for quantity Q has 'set' prepended and the getter is just the quantity,
i.e.,
setQ()andq()(note lower case first letter for the getter).
- Follow CMSSW standards such that the setter for quantity Q has 'set' prepended and the getter is just the quantity,
i.e.,
- Class names start with upper-case letter. Variables & functions start with lower-case letter.
- Avoid lots of almost identical lines of code. Or almost duplicate code. Keep code short.
- Add plenty of comments, especially when the code obscure, but even if it seems clear to you.
- The class definition file should contain a short description of the class at the start of the file
- Put class implementation in separate file (.cc) to declaration (.h).
- Or put at end of .h file if templated classes prevent use of .cc.
- Git PRs reviewed by another person before merge to master.
- this is technically enforced in this repository
- Do not use
#pragma onceto avoid duplicate inclusion of header files. Instead, use include guards with the following format:
#ifndef SubdirectoryName_FileName_h
#define SubdirectoryName_FileName_h
…
#endif- General constants start with k. e.g.
kNumEventsand are declaredconstexpr, or "static const" where this doesn't work. - Constants specifying number of bits are defined in the relevant memory header file, start with 'k' and end with 'Size', i.e.,
kChi2Size- alternate proposal: start with B & capitalized. e.g
BCHI2?
- alternate proposal: start with B & capitalized. e.g
- Size constants are declared via enum
- Don't use
static' variables unless they areconst', since if multiple copies of an algo block are created, they will cause cross-talk between them. - Use of class templates can avoid code duplication.
- Declare variables in the deepest scope possible to help the HLS compiler better understand data dependencies. It may help to minimize hardware usage because the compiler does not need to preserve the variable data across loops.
- Avoid
breakandcontinuestatement in loops. It's very expensive to usebreakandcontinuestatements in terms of both hardware resource and timing - Consider avoiding nested if-else statements in loops, if this can be done without introducing more complicated logic. Nested if-else statements inside loops can be expensive because such statements are synthesized as combinational logic and cannot be parallelized. One way is to replace if-statements with the conditional operation, i.e. instead of:
int zbin = stubz;
if(start!=ibin) zbin += 8;re-write this as:
int zbin = (start!=ibin) ? stubz+8 : stubz;Using conditional operation will enable the HLS compiler to use a simple multiplexer to assign value to zbin, instead of generating complex combinational control logic.
- Re-order code to perform load operations in parallel. For example, from the following code, the HLS compiler will schedule the loading of array values to be sequential,
bool isSkip = arr1[i] || arr2[j] || arr3[k];whereas the following will enable the compiler to schedule the array accesses in parallel,
bool tmp1 = arr1[i];
bool tmp2 = arr2[j];
bool tmp3 = arr3[j];
bool isSkip = tmp1 || tmp2 || tmp3;We use the following model for the inter-module RAMs in the project.
- In a header file we define both the data that the RAM contains as well as the particular template instantiation that defines the RAM. For instance, following the
StubPairMemory.hexample. In the header file we define a classStubPairthat is the data stored in aStubPairMemory, which in turn is just the instantiation of the templateMemoryBasewith aStubPairdata member.
For the data types:
- data is stored in aggregate types (packed)
- getters and setters provide access to the physically meaningful quantities from these bitpacked types.
The data types follow a strict convention and were all written by the same person (Derek). The header file first defines via enums the bit sizes of each piece of information, for example from the TrackletParameters data type:
enum BitWidths {
// Bit sizes for TrackletParameterMemory fields
kTParRinvSize = 14, //rinv
kTParPhi0Size = 18, //phi0
kTParZ0Size = 10, //z0
kTParTSize = 14, //t
// Bit size for full TrackletParameterMemory
kTrackletParameterSize = kTParRinvSize + kTParPhi0Size + kTParZ0Size + kTParTSize + 2*kNBits_MemAddr
};From these definitions the LSB and MSB of the packed bitword is calculated by the compiler and then used in the getter and setter routines, so if bit sizes change only this enum needs to be updated.