Skip to content

[LibertyClk] Support for Liberty file defined generated clock#378

Draft
markram1729 wants to merge 11 commits intoparallaxsw:masterfrom
markram1729:exLibertyClk
Draft

[LibertyClk] Support for Liberty file defined generated clock#378
markram1729 wants to merge 11 commits intoparallaxsw:masterfrom
markram1729:exLibertyClk

Conversation

@markram1729
Copy link
Copy Markdown

@markram1729 markram1729 commented Feb 8, 2026

from silimate Opensta pull Adds support for Liberty file-defined generated clocks in OpenSTA, including new classes, functions, and tests.

  • Behavior:
    • Adds support for Liberty file-defined generated clocks in LibertyReader.cc and Liberty.cc.
    • Introduces GeneratedClock class in GeneratedClock.hh and GeneratedClock.cc to represent generated clocks.
    • Updates Sta class in Sta.cc to handle generated clocks, including functions like updateGeneratedClks() and setUpdateGenclks().
  • Functions:
    • Adds makeGeneratedClock() in Liberty.cc to create generated clock objects.
    • Implements createLibertyGeneratedClocks() in Sdc.cc to create generated clocks from Liberty definitions.
    • Modifies VerilogReader.cc to map generated clock pins to Liberty cells.
  • Tests:
    • Adds test files generated_clock.lib, generated_clock.tcl, generated_clock.v, and generated_clock.ok to verify generated clock functionality.
    • Updates regression_vars.tcl to include generated_clock test.

@markram1729
Copy link
Copy Markdown
Author

I will make changes to the formatting that is suitable for opensta

@jjcherry56
Copy link
Copy Markdown
Collaborator

The first step is to remove all of the formatting changes that completely obscure the functional changes.

* fix : compilation

* fix tcl commands

* Update Network.cc
Comment thread sdc/Clock.cc Outdated
static bool
isPowerOfTwo(int i)
bool
isPowerofTwo(int i)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

restore the original name that uses camel case

Comment thread include/sta/Sdc.hh
float fanout);
void setMaxArea(float area);
float maxArea() const;
void createLibertyGeneratedClocks(Clock *clk);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

use make instead of create for naming consistency with the rest of the code


namespace sta {

class GeneratedClock
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The name GeneratedClock is too generic, since there are already generated clocks in sdc.
I suggest LibertyGenClk. The other functions names should be updated to be consistent.

{
public:
~GeneratedClock();
const char *name() const { return name_; }
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

use std::string

Comment thread include/sta/Liberty.hh

// Build helpers.
void makeGeneratedClock(const char *name,
const char *clock_pin,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

use spaces for indentation (this changed with rel 3.0)

int line_;
};

class GeneratedClockGroup
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You don't need GeneratedClockGroup; you could use a GeneratedClock that is
not in the Liberty and then add it. But the state class will be unnecessary
with the new liberty reader.

Comment thread sdc/Sdc.cc Outdated
if (sta->pins(clk)) {

// All pins along the clock network
const PinSet clkNetworkPins = *sta->pins(clk);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

the variable is named like a function; use snake case

Comment thread sdc/Sdc.cc Outdated
// The keys of generated_clock_pins_to_cells_
// (master clock pins) will be searched in the current clock network
for (const auto &entry : generated_clock_pins_to_cells) {
const char *pinName = entry.first;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

use c++17 [pin_name, cell] syntax to avoid first and second.

pinName should be pin_name.
See doc/CodingGuidelines.txt

Comment thread sdc/Sdc.cc
clkHpinDisablesInvalid();

// Trigger update of generated clocks
Sta::sta()->setUpdateGenclks();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

again, wrong to referencd Sta::sta()

Comment thread tcl/CmdArgs.tcl Outdated
return $clks
}

# Get attribute
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this has nothing to do with generated clocks and has no business being here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Resolved

Comment thread tcl/CmdUtil.tcl Outdated
define_cmd_args "get_name" {object}
define_cmd_args "get_full_name" {object}

#Get Object name
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

remove

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

done

Comment thread tcl/CmdUtil.tcl Outdated
define_cmd_args "get_full_name" {object}

#Get Object name
interp alias {} get_object_name {} get_full_name
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

put this in your .sta, not the sources

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

resolved

Comment thread tcl/CmdUtil.tcl Outdated
}

proc get_full_name { object } {
if { [llength $object] > 1 } {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

remove

Comment thread verilog/VerilogReader.cc

// Maps a clock pin path to the liberty cell containing the generated clock definition
void
VerilogReader::makeGeneratedClocks(LibertyCell *lib_cell, Instance *inst)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

VerilogReader::makeGeneratedClocks makes no sense and violates modularity.
Generated clocks should work with a database that does not use the
verilog reader.

@jjcherry56
Copy link
Copy Markdown
Collaborator

The Liberty reader is being rewritten so the parsing part of this will change rather drastically. It will probably be a week or so before those changes are pushed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants