- Primitive Naming Convention
- Transparent Latches
- Asynchronous Resets
- Registers Without Resets
- Critical Clock Domain Crossing Issues
- Design Policy Checks Summary
- References
Much of what follows is subjective and will be the subject of local design policies rather than being hard and fast right or wrong. That said, let's start with an easy design check that we are constantly reminded about in good design guides.
Primitive Naming Convention
A quick note on the naming convention that seems to be used by Xilinx's Vivado. The PRIMITIVE_TYPE property is the concatenation of three others: PRIMITIVE_GROUP, PRIMITIVE_SUBGROUP and REF_NAME. I'm trying to use this in order to try and keep the TCL functions device independent.
If: PRIMITIVE_GROUP = REGISTER PRIMITIVE_SUBGROUP = SDR REF_NAME = FDPE Then: PRIMITIVE_TYPE = REGISTER.SDR.FDPE
Transparent Latches
Typically induced by incomplete assignments in combinatorial processes, even the wary can be caught out in non-obvious situations. A hard fault to spot for example is when a variable or signal is assigned within a case statement, and that case statement does not assign the signal or variable in all possible paths. Its hard to spot what's not there in the code, hence it will still catch out the most wary.
# Transparent latches - Worthy of ritual embarrassment
# ILD* Macro: Transparent Input Data Latch [with *]
# LDCE/LDPE Macros for Ultrascale
proc get_transparent_latches {{verbose 0}} {
set ret {}
foreach r [get_cells -quiet -hierarchical -filter {PRIMITIVE_TYPE =~ "REGISTER.LATCH*"}] {
lappend ret $r
if {$verbose} {
puts "Warning - $r is a transparent latch."
}
}
return $ret
}
Xilinx UG799 lists a number of primitives providing the functionality of a transparent latch, all beginning with ILD*, e.g. the ILDI_1 Macro: Transparent Input Data Latch with Inverted Gate (Asynchronous Preset).
Asynchronous Resets
Xilinx Forum article "Demystifying Resets: Synchronous, Asynchronous other Design Considerations... Part 1" amongst others makes the case for synchronous resets to be used in Xilinx FPGAs. The main concern about reset signals is when they are de-asserted. The removal of the reset relative to any clock edge must not violate any setup and hold times for the clock edge in order to avoid risking metastability. You would not expect to see any asynchronous resets in an FPGA design, they are more typically found in ASICs.
# Asynchronous resets (Xilinx FPGAs like synchronous...)
# Ref: https://forums.xilinx.com/t5/PLD-Blog-Archived/That-Dangerous-Asynchronous-Reset/ba-p/12856
# FDCE Primitive: D Flip-Flop with Asynchronous Clear
# FDPE Primitive: D Flip-Flop with Asynchronous Preset
proc get_async_reset_registers {{verbose 0}} {
set ret {}
foreach r [get_cells -quiet -hierarchical -filter {PRIMITIVE_TYPE =~ "REGISTER.SDR.FDC*" || PRIMITIVE_TYPE =~ "REGISTER.SDR.FDP*"}] {
lappend ret $r
if {$verbose} {
puts "Warning - $r has an asynchronous reset."
}
}
return $ret
}
The synchronous (p)reset primitives are FDR* and FDS* plus the negative edge companions. If we find any FDP* or FDC* primitives then an asynchronous reset has been coded.
Registers Without Resets
This is less about finding a clear mistake since there are times when resets are deliberately omitted in order to code sympathetically for an SRL16 primitive (see SRL Inferencing with Xilinx FPGAs). Several of the references at the end of this article suggest a reset is not actually necessary, e.g. "because a Xilinx FPGA already starts up in a known state following configuration, the global reset is really not necessary" [WP275]. A simple rule would be "data paths do not need reset if qualified by a “valid” signal that is reset." Therefore detecting the absence of resets on registers is not automatically a design flaw, but we have all at some point forgotten to add signals that are intended to be reset to the reset clause before. There are also examples where a coded reset clause in VHDL produces a register with its reset tied, probably as a synchronous reset gets realised in logic via a LUT, so this is looking like an even less useful check after all.
# Paired with 'get_no_reset_registers'.
proc has_tied_reset {cell} {
set pin [get_pins -quiet -filter {IS_CLEAR || IS_PRESET || IS_RESET || IS_SET || IS_SETRESET} -of_objects [get_cells $cell]]
if {[llength $pin] > 0} {
# get_property IS_CONNECTED $pin is true for resets tied to GND/VCC
return [get_property IS_TIED $pin]
} else {
if {[llength $cell] > 0} {
# $cell has no set, reset, clear, or preset pin
return 2
} else {
error "No cell parameter specified."
}
}
}
# Registers with no reset (policy?). Means no reset signal just tied high or low permanently.
proc get_no_reset_registers {{verbose 0}} {
set ret {}
foreach r [all_registers] {
if {[has_tied_reset $r] == 0} {
lappend ret $r
if {$verbose} {
puts "Warning - $r has tied reset, not reset by a signal. Indicates a missing reset clause in a clocked process."
}
}
}
return $ret
}
Critical Clock Domain Crossing Issues
This is a tough one and really requires a more manual inspection. It would be ideal to identify failures to set the ASYNC_REG property on a pair of double re-time registers and the false path constraint driving the first of the pair. This solution is more general and just summarises and filters the existing report_cdc TCL command in Vivado.
# Find all critical clock domain crossing issues
#
# Usage: get_problem_cdc_issues 1
#
# Understanding ASYNC_REG attribute
# https://forums.xilinx.com/t5/Timing-Analysis/Understanding-ASYNC-REG-attribute/td-p/774023
# Verify "unsafe" column in report_cdc
#
proc get_problem_cdc_issues {{verbose 0}} {
report_cdc -quiet
set ret [get_cdc_violations -filter {SEVERITY == Critical}]
if {$verbose} {
foreach cdc $ret {
if {![get_property IS_WAIVED $cdc]} {
puts "[format "#%03d: " [get_property ID $cdc]] [get_property DESCRIPTION $cdc]"
puts " Severity: [get_property SEVERITY $cdc]"
puts " Clock Crossing: [get_property STARTPOINT_CLOCK $cdc] -> [get_property ENDPOINT_CLOCK $cdc]"
puts " Start Pin: [get_property STARTPOINT_PIN $cdc]"
puts " End Pin: [get_property ENDPOINT_PIN $cdc]"
puts " Check: [get_property CHECK $cdc]"
puts " Exception: [get_property EXCEPTION $cdc]"
}
}
}
return $ret
}
Design Policy Checks Summary
The above checks can be pulled together for a simple summary as part of a standard compilation script to provide some design insights before considering the design finished. This might save you some embarrassment from others in your team!
# Display a summary of the identified design issues we care about chastising designer for.
#
# Usage: design_policy_checks
#
proc design_policy_checks {} {
set arr [get_async_reset_registers]
if {[llength $arr] > 0} {
puts "Warning - [llength $arr] with asynchronous reset. Run 'get_async_reset_registers 1' for details."
}
set tl [get_transparent_latches]
if {[llength $tl] > 0} {
puts "Warning - [llength $tl] transparent latches. Run 'get_transparent_latches 1' for details. Sack the code author."
}
set cdc [get_problem_cdc_issues]
if {[llength $cdc] > 0} {
puts "Warning - [llength $cdc] Critical clock domain crossing (CDC) issues. Run 'get_problem_cdc_issues 1' or 'report_cdc' for details."
}
}
design_policy_checks
References
- Github Source Code
- Xilinx 7 Series FPGA and Zynq-7000 All Programmable SoC Libraries Guide for Schematic Designs, UG799 (v14.7) October 2, 2013
- Xilinx Forum: Demystifying Resets: Synchronous, Asynchronous other Design Considerations... Part 1
- Xilinx Forum: That Dangerous Asynchronous Reset!
- Get your Priorities Right – Make your Design Up to 50% Smaller, WP275 (v1.0.1) October 22, 2007
- HDL Coding Practices to Accelerate Design Performance, WP231 (1.1) January 6, 2006