Skip to content

Add support for falling edge clocking #2211

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 55 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
638a476
Libarchfpga: add 'edge' attribute to <port> under <model>
lpawelcz Oct 28, 2022
00c53c0
Tatum: store triggering edge info in TimingGraph
lpawelcz Nov 15, 2022
b9d570d
TimingGraph builder: write triggering edge info to TimingGraph
lpawelcz Nov 23, 2022
af81901
Tatum: show triggering clock edge in TimingGraph visualization
lpawelcz Nov 23, 2022
b4af78f
vpr: base: read_blif: add support for parsing falling edge clocked FFs
lpawelcz Nov 17, 2022
c653664
vpr: base: atom_netlist_utils: add support for printing netlist with …
lpawelcz Nov 23, 2022
f1089f7
Libarchfpga: add internal falling edge clocked .latch model
lpawelcz Nov 21, 2022
4253fac
vpr: test_interchange_device: expect second latch model in internal l…
lpawelcz Dec 9, 2022
560e5ee
vpr: utils: add second variant of primitive_type_feasible() check
lpawelcz Nov 23, 2022
a82d8d4
vpr: pack: use second primitive_type_feasible() check
lpawelcz Dec 7, 2022
a1f7c54
libarchfpga: add secondary models to t_pb_type and t_pb_graph_node
lpawelcz Dec 7, 2022
725bc6b
libarchfpga: SyncModelsPbTypes: sync secondary models
lpawelcz Dec 7, 2022
9033994
vpr: utils: use primary or secondary pins
lpawelcz Dec 7, 2022
c1004c1
pack: pb_type_graph: use primary or secondary ports
lpawelcz Dec 7, 2022
4df34e1
base: read netlist: always assign primary pins
lpawelcz Dec 7, 2022
55c6c47
base: read netlist: fixup asserts
lpawelcz Dec 7, 2022
04cb06c
netlist writer: handle triggering clock edge in post-impl SDF writer
lpawelcz Nov 23, 2022
2653d83
[TEST] EArch.xml: showcase 'edge' attribute in clock ports
lpawelcz Nov 23, 2022
76b9203
[TEST] multiclock.blif: set one FF as clocked on falling edge
lpawelcz Nov 23, 2022
b63083b
add 180 deg shifted clocks derived from netlist clock in sdc file
lpawelcz Dec 15, 2022
b26fbaa
libsdcparse: keep data on inverted clocks
lpawelcz Dec 19, 2022
5e4a8f6
libtatum: skip incompatible constraints in timing tags propagation
lpawelcz Dec 19, 2022
c28d73b
vpr: timing: read_sdc: create clock domains with data on inverted clocks
lpawelcz Dec 19, 2022
9e566eb
vpr: timing: read_sdc: add special case for inverted clocks
lpawelcz Dec 20, 2022
b0adc09
invert rise- and fall-edge times for virtual inverted clocks
lpawelcz Dec 21, 2022
b592f23
add_sdc_create_clock: add comments
lpawelcz Dec 21, 2022
5d297ec
domain_inverted: fix indent
lpawelcz Dec 21, 2022
b177df7
libtatum: TimingConstraints: document clock_domain_inverted()
lpawelcz Dec 21, 2022
c1b127c
libtatum: TimingConstraints: fix typo
lpawelcz Dec 21, 2022
9a0467f
libtatum: use enum for triggering edges
lpawelcz Dec 21, 2022
5b47ceb
timing_graph_builder: convert enum types
lpawelcz Dec 21, 2022
2a4f193
libtatum: CommonAnalysisVisitor: comment the tag propagation mechanism
lpawelcz Dec 21, 2022
079efe0
libarchfpga: arch_util: remove magic number for internal library size
lpawelcz Dec 22, 2022
ab19fee
libarchfpga: arch_util: change all free() to delete
lpawelcz Dec 22, 2022
2353564
libarchfpga: change all .alloc() calls to 'new'
lpawelcz Dec 22, 2022
e0a3b93
libarchfpga: comments for freeing internal models lib
lpawelcz Dec 22, 2022
362821a
arch_util: cleanup comments
lpawelcz Dec 22, 2022
0ce6545
libarchfpga: add enum for internal models
lpawelcz Dec 22, 2022
5c76fcb
libarchfpga: comments for module library latch duplication
lpawelcz Dec 22, 2022
7f9c9b0
libarchfpga: physical_types: comment logical models
lpawelcz Dec 23, 2022
d859ec3
libarchfpga: physical_types: t_port: remove trigg_edge
lpawelcz Dec 23, 2022
de3857d
libarchfpga: physical_types: comment ports
lpawelcz Dec 23, 2022
977b9d6
libarchfpga: t_pb_type: remove num_ports_sec
lpawelcz Dec 23, 2022
3e4cd85
libarchfpga: physical_types: comment update_pins
lpawelcz Dec 23, 2022
91369c9
vpr_utils: comment primitive_type_feasible
lpawelcz Dec 23, 2022
ebfaea8
libarchfpga: read xml: comments on processing secondary pb_type ports
lpawelcz Dec 23, 2022
4649f0f
vpr: atom_netlist_utils: fixup latch handling in printing netlist
lpawelcz Dec 23, 2022
cb46900
libarchfpga: models library: add constant for the ID of latch clock i…
lpawelcz Dec 23, 2022
50fe438
vpr: pack: pb_type_graph: explain has_secondary usage
lpawelcz Dec 23, 2022
9996637
vpr: timing: read_sdc: comment get_clocks
lpawelcz Dec 23, 2022
f6e7fea
vpr_utils: explain TriggeringEdge usage in pb_graph_pin matching
lpawelcz Dec 23, 2022
e947026
libarchfpga: physical_types: link to t_pb_types definition
lpawelcz Dec 23, 2022
4ed6e5a
formatting fixes
lpawelcz Dec 23, 2022
aa0561b
Revert "libarchfpga: change all .alloc() calls to 'new'"
lpawelcz Dec 23, 2022
c59f504
Revert "libarchfpga: arch_util: change all free() to delete"
lpawelcz Dec 23, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions libs/EXTERNAL/libsdcparse/src/sdc_common.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,31 @@ void add_sdc_create_clock(Callback& callback, const Lexer& lexer, CreateClock& s
*/
callback.create_clock(sdc_create_clock);

//Prepare and create inverted version of the netlist clock.
//Use netlist clock from parsed SDC file (create_clock command)
//as a base for the inverted clock. It is created as virtual
//so that it won't interfere with existing netlist clock.
sdc_create_clock.is_virtual = true;
//Mark clock as inverted
sdc_create_clock.inverted = true;

//Virtual clocks must have empty targets, overwrite those
//but first save the names of the targets
auto targets = sdc_create_clock.targets.strings;
sdc_create_clock.targets = StringGroup();

//180 degrees phase shift is done with inverting
//the rise- and fall-edge times of the original netlist clock
double rise_edge = sdc_create_clock.rise_edge;
sdc_create_clock.rise_edge = sdc_create_clock.fall_edge;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding @kmurray so he can take a look.

Is this creating a half cycle clock transfer from -edge to +edge?
Would be good to comment that. Also possibly we should be using launch and capture edge terminology (or active edge) as rise_edge = fall_edge is a bit mysterious looking. So consider some variable renaming. We should at least comment it very thoroughly!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comments describing the inversion. I'm not sure if renaming rise_edge and fall_edge from sdcparse::CreateClock is necessary because those precisely describe the waveform of the clock.
I also slightly modified the 180 degree phase shift.

sdc_create_clock.fall_edge = rise_edge;

//Create new inverted virtual clock for each target from original netlist clock.
for (auto &str : targets) {
//Use saved names of the original netlist clock targets as inverted clock names
sdc_create_clock.name = str + "_negedge";
callback.create_clock(sdc_create_clock);
}
}

/*
Expand Down
1 change: 1 addition & 0 deletions libs/EXTERNAL/libsdcparse/src/sdcparse.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ struct CreateClock {
StringGroup targets; //The set of strings indicating clock sources.
// May be explicit strings or regexs.
bool is_virtual = false; //Identifies this as a virtual (non-netlist) clock
bool inverted = false; //Identifies this as inverted version of netlist clock
};

struct SetIoDelay {
Expand Down
25 changes: 24 additions & 1 deletion libs/EXTERNAL/libtatum/libtatum/tatum/TimingConstraints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ std::string TimingConstraints::clock_domain_name(const DomainId id) const {
return domain_names_[id];
}

bool TimingConstraints::clock_domain_inverted(const DomainId id) const {
return domain_inverted_[id];
}

NodeId TimingConstraints::clock_domain_source_node(const DomainId id) const {
return domain_sources_[id];
}
Expand Down Expand Up @@ -292,7 +296,8 @@ DomainId TimingConstraints::create_clock_domain(const std::string name) {
//Create it
id = DomainId(domain_ids_.size());
domain_ids_.push_back(id);


domain_inverted_.push_back(false);
domain_names_.push_back(name);
domain_sources_.emplace_back(NodeId::INVALID());

Expand All @@ -303,6 +308,24 @@ DomainId TimingConstraints::create_clock_domain(const std::string name) {
return id;
}

DomainId TimingConstraints::create_clock_domain(const std::string name, bool inverted) {
DomainId id = find_clock_domain(name);
if(!id) {
//Create it
id = DomainId(domain_ids_.size());
domain_ids_.push_back(id);

domain_inverted_.push_back(inverted);
domain_names_.push_back(name);
domain_sources_.emplace_back(NodeId::INVALID());

TATUM_ASSERT(clock_domain_name(id) == name);
TATUM_ASSERT(find_clock_domain(name) == id);
}

return id;
}

void TimingConstraints::set_setup_constraint(const DomainId src_domain, const DomainId sink_domain, const Time constraint) {
set_setup_constraint(src_domain, sink_domain, NodeId::INVALID(), constraint);
}
Expand Down
9 changes: 8 additions & 1 deletion libs/EXTERNAL/libtatum/libtatum/tatum/TimingConstraints.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,15 @@ class TimingConstraints {
///\returns The name of a clock domain
std::string clock_domain_name(const DomainId id) const;

///Checks whether the clock domain was marked as inverted. Inverted clocks are virtual clocks created from netlist clocks with 180 degree phase shift applied
///\param id The ID of the clock domain
///\returns whether the clock domain with specified domain id was marked as inverted
bool clock_domain_inverted(const DomainId id) const;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment saying what this does (can be short, but should explain what an inverted clock implies). Should be doxygen style, like the rest of the comments in this file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added


///\returns The source NodeId of the specified domain
NodeId clock_domain_source_node(const DomainId id) const;

//\returns whether the specified domain id corresponds to a virtual lcock
//\returns whether the specified domain id corresponds to a virtual clock
bool is_virtual_clock(const DomainId id) const;

///\returns The domain of the specified node id if it is a clock source
Expand Down Expand Up @@ -123,6 +128,7 @@ class TimingConstraints {
public: //Mutators
///\returns The DomainId of the clock with the specified name (will be created if it doesn not exist)
DomainId create_clock_domain(const std::string name);
DomainId create_clock_domain(const std::string name, bool inverted);

///Sets the setup constraint between src_domain and sink_domain with value constraint
void set_setup_constraint(const DomainId src_domain, const DomainId sink_domain, const Time constraint);
Expand Down Expand Up @@ -170,6 +176,7 @@ class TimingConstraints {
private: //Data
tatum::util::linear_map<DomainId,DomainId> domain_ids_;
tatum::util::linear_map<DomainId,std::string> domain_names_;
tatum::util::linear_map<DomainId, bool> domain_inverted_;
tatum::util::linear_map<DomainId,NodeId> domain_sources_;

std::unordered_set<NodeId> constant_generators_;
Expand Down
30 changes: 29 additions & 1 deletion libs/EXTERNAL/libtatum/libtatum/tatum/TimingGraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,32 @@ NodeId TimingGraph::add_node(const NodeType type) {
return node_id;
}

NodeId TimingGraph::add_node(const NodeType type, TriggeringEdge trigg_edge) {
//Invalidate the levelization
is_levelized_ = false;

//Reserve an ID
NodeId node_id = NodeId(node_ids_.size());
node_ids_.push_back(node_id);

//Type
node_types_.push_back(type);

//Triggering Edge
trigg_edges_.push_back(trigg_edge);

//Edges
node_out_edges_.push_back(std::vector<EdgeId>());
node_in_edges_.push_back(std::vector<EdgeId>());

//Verify sizes
TATUM_ASSERT(node_types_.size() == node_out_edges_.size());
TATUM_ASSERT(node_types_.size() == node_in_edges_.size());

//Return the ID of the added node
return node_id;
}

EdgeId TimingGraph::add_edge(const EdgeType type, const NodeId src_node, const NodeId sink_node) {
//We require that the source/sink node must already be in the graph,
// so we can update them with thier edge references
Expand Down Expand Up @@ -556,6 +582,7 @@ void TimingGraph::remap_nodes(const tatum::util::linear_map<NodeId,NodeId>& node
node_types_ = clean_and_reorder_values(node_types_, node_id_map);
node_in_edges_ = clean_and_reorder_values(node_in_edges_, node_id_map);
node_out_edges_ = clean_and_reorder_values(node_out_edges_, node_id_map);
trigg_edges_ = clean_and_reorder_values(trigg_edges_, node_id_map);

//Update references
edge_src_nodes_ = update_all_refs(edge_src_nodes_, node_id_map);
Expand Down Expand Up @@ -597,7 +624,8 @@ bool TimingGraph::validate_sizes() const {
if ( node_ids_.size() != node_types_.size()
|| node_ids_.size() != node_in_edges_.size()
|| node_ids_.size() != node_out_edges_.size()
|| node_ids_.size() != node_levels_.size()) {
|| node_ids_.size() != node_levels_.size()
|| node_ids_.size() != trigg_edges_.size()) {
throw tatum::Error("Inconsistent node attribute sizes");
}

Expand Down
34 changes: 32 additions & 2 deletions libs/EXTERNAL/libtatum/libtatum/tatum/TimingGraph.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,13 @@

namespace tatum {

enum TriggeringEdge {
RISING_EDGE,
FALLING_EDGE,
DONT_CARE,
ERROR_EDGE
};

class TimingGraph {
public: //Public types
//Iterators
Expand All @@ -84,6 +91,29 @@ class TimingGraph {
///\returns The type of the node
NodeType node_type(const NodeId id) const { return node_types_[id]; }

///\param id The TimingGraph node ID
///\returns TriggeringEdge enum describing the triggering edge of the TimingGraph node
TriggeringEdge trigg_edge(const NodeId id) const { return trigg_edges_[id]; }

///\param id The TimingGraph node ID
///\returns String describing the triggering edge of the TimingGraph node
std::string trigg_edge_str(const NodeId id) const {
switch(trigg_edges_[id]) {
case TriggeringEdge::RISING_EDGE:
return "RISING";
break;
case TriggeringEdge::FALLING_EDGE:
return "FALLING";
break;
case TriggeringEdge::DONT_CARE:
return "DONT_CARE";
break;
default:
return "ERROR";
break;
}
}

///\param id The node id
///\returns A range of all out-going edges the node drives
edge_range node_out_edges(const NodeId id) const { return tatum::util::make_range(node_out_edges_[id].begin(), node_out_edges_[id].end()); }
Expand Down Expand Up @@ -198,6 +228,7 @@ class TimingGraph {
///\param type The type of the node to be added
///\warning Graph will likely need to be re-levelized after modification
NodeId add_node(const NodeType type);
NodeId add_node(const NodeType type, TriggeringEdge trigg_edge);

///Adds an edge to the timing graph
///\param type The edge's type
Expand Down Expand Up @@ -282,6 +313,7 @@ class TimingGraph {
//Node data
tatum::util::linear_map<NodeId,NodeId> node_ids_; //The node IDs in the graph
tatum::util::linear_map<NodeId,NodeType> node_types_; //Type of node
tatum::util::linear_map<NodeId,TriggeringEdge> trigg_edges_; //Triggering edge of the clock
tatum::util::linear_map<NodeId,std::vector<EdgeId>> node_in_edges_; //Incomiing edge IDs for node
tatum::util::linear_map<NodeId,std::vector<EdgeId>> node_out_edges_; //Out going edge IDs for node
tatum::util::linear_map<NodeId,LevelId> node_levels_; //Out going edge IDs for node
Expand Down Expand Up @@ -334,7 +366,5 @@ struct GraphIdMaps {
tatum::util::linear_map<EdgeId,EdgeId> edge_id_map;
};



} //namepsace

Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,7 @@ bool CommonAnalysisVisitor<AnalysisOps>::do_arrival_traverse_edge(const TimingGr

//Pulling values from upstream source node
NodeId src_node_id = tg.edge_src_node(edge_id);
NodeId sink_node_id = tg.edge_sink_node(edge_id);

if(should_propagate_clocks(tg, tc, edge_id)) {
/*
Expand All @@ -311,7 +312,6 @@ bool CommonAnalysisVisitor<AnalysisOps>::do_arrival_traverse_edge(const TimingGr

for(const TimingTag& src_launch_clk_tag : src_launch_clk_tags) {
//Standard propagation through the clock network

Time new_arr = src_launch_clk_tag.time() + clk_launch_edge_delay;
timing_modified |= ops_.merge_arr_tags(node_id, new_arr, src_node_id, src_launch_clk_tag);

Expand All @@ -328,6 +328,28 @@ bool CommonAnalysisVisitor<AnalysisOps>::do_arrival_traverse_edge(const TimingGr

for(const TimingTag& src_capture_clk_tag : src_capture_clk_tags) {
//Standard propagation through the clock network

/*
* Each source capture tag may be related to clock domain of one of the two types:
* 1. clock domain created for netlist clock, meant to be used with cells clocked at the rising edge
* 2. inverted virtual clock domain based on existing netlist clock.
* It is 180 degree phase shifted in relation to netlist clock
* and it is used in timing analysis for cells clocked at falling edge.
*
* The triggering edges of FFs should be taken into account when propagating tags
* to CPIN nodes. Tags of types which are incompatible with triggering edges
* of the FF clock inputs shouldn't be propagated because having those wouldn't allow
* timing analysis to calculate correct timings for transfers between cells
* clocked rising and falling edges of the same clock.
*/
if ( tg.node_type(sink_node_id) == NodeType::CPIN ) {
if ((tg.trigg_edge(sink_node_id) == TriggeringEdge::FALLING_EDGE && !tc.clock_domain_inverted(src_capture_clk_tag.launch_clock_domain())) ||
(tg.trigg_edge(sink_node_id) == TriggeringEdge::RISING_EDGE && tc.clock_domain_inverted(src_capture_clk_tag.launch_clock_domain()))) {
//Skip propagation of timings derived from incompatible constraints
continue;
}
}

timing_modified |= ops_.merge_arr_tags(node_id, src_capture_clk_tag.time() + clk_capture_edge_delay, src_node_id, src_capture_clk_tag);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ void GraphvizDotWriter::write_dot_node(std::ostream& os,
os << "\t";
os << node_name(node);
os << "[label=\"";
os << "{" << node << " (" << tg_.node_type(node) << ")";
os << "{" << node << " (" << tg_.node_type(node) << ") TEdge(" << tg_.trigg_edge_str(node) << ")";

for(const auto& tag_set : {tags, slacks}) {
for(const auto& tag : tag_set) {
Expand Down
Loading