Skip to content
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

odb: protecting LefParser and DefParser namespaces #6475

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
2 changes: 0 additions & 2 deletions src/OpenRoad.i
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@
%{

#include "odb/db.h"
#include "odb/lefin.h"
#include "odb/defin.h"
#include "odb/defout.h"
#include "sta/Report.hh"
#include "sta/Network.hh"
Expand Down
1 change: 0 additions & 1 deletion src/Tech.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@

#include "db_sta/dbSta.hh"
#include "odb/db.h"
#include "odb/lefin.h"
#include "ord/OpenRoad.hh"

namespace ord {
Expand Down
2 changes: 0 additions & 2 deletions src/gui/src/gui.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,7 @@
#include "mainWindow.h"
#include "odb/db.h"
#include "odb/dbShape.h"
#include "odb/defin.h"
#include "odb/geom.h"
#include "odb/lefin.h"
#include "ord/OpenRoad.hh"
#include "ruler.h"
#include "scriptWidget.h"
Expand Down
2 changes: 1 addition & 1 deletion src/odb/include/odb/dbStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ class dbIStream
return *this;
} else {
*this >> std::get<I>(tup);
return ((*this).operator>> <I + 1>(tup));
return ((*this).operator>><I + 1>(tup));
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/odb/include/odb/defin.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

#pragma once

#include <mutex>
#include <vector>

#include "odb.h"
Expand All @@ -51,6 +52,7 @@ class dbTech;
class defin
{
definReader* _reader;
static std::mutex _def_mutex;

public:
enum MODE
Expand Down
44 changes: 40 additions & 4 deletions src/odb/include/odb/lefin.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#pragma once

#include <list>
#include <mutex>
#include <string>
#include <vector>

Expand Down Expand Up @@ -65,7 +66,6 @@ class lefiObstruction;
class lefiGeometries;
struct lefiGeomPolygon;
} // namespace LefParser

namespace odb {

class dbObject;
Expand All @@ -77,7 +77,7 @@ class dbDatabase;
class dbTechLayer;
class dbSite;

class lefin
class lefinReader
{
dbDatabase* _db;
dbTech* _tech;
Expand Down Expand Up @@ -196,8 +196,10 @@ class lefin
}
void lineNumber(int lineNo);

lefin(dbDatabase* db, utl::Logger* logger, bool ignore_non_routing_layers);
~lefin();
lefinReader(dbDatabase* db,
utl::Logger* logger,
bool ignore_non_routing_layers);
~lefinReader();

// Skip macro-obstructions in the lef file.
void skipObstructions() { _skip_obstructions = true; }
Expand Down Expand Up @@ -241,4 +243,38 @@ class lefin
bool updateTechAndLib(dbLib* lib, const char* lef_file);
};

class lefin
{
public:
lefin(dbDatabase* db, utl::Logger* logger, bool ignore_non_routing_layers);
~lefin();

// convert distance value to db-units
int dbdist(double value);

// Create a technology from the tech-data of this LEF file.
dbTech* createTech(const char* name, const char* lef_file);

// Create a library from the library-data of this LEF file.
dbLib* createLib(dbTech* tech, const char* name, const char* lef_file);

// Create a technology and library from the MACRO's in this LEF file.
dbLib* createTechAndLib(const char* tech_name,
const char* lib_name,
const char* lef_file);

// Add macros to this library
bool updateLib(dbLib* lib, const char* lef_file);

// Update a technology from the tech-data of this LEF file.
bool updateTech(dbTech* tech, const char* lef_file);

// Add macros to this library and the technology of this library
bool updateTechAndLib(dbLib* lib, const char* lef_file);

private:
lefinReader* _reader;
static std::mutex _lef_mutex;
};

} // namespace odb
17 changes: 17 additions & 0 deletions src/odb/src/defin/defin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,70 +37,85 @@

namespace odb {

std::mutex defin::_def_mutex;

defin::defin(dbDatabase* db, utl::Logger* logger, MODE mode)
{
std::lock_guard<std::mutex> lock(_def_mutex);
_reader = new definReader(db, logger, mode);
}

defin::~defin()
{
std::lock_guard<std::mutex> lock(_def_mutex);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not clear the locking is doing anything here. It kind of looks like this is trying to protect two defin's from running at the same time on the same database, but what we're really concerned about is two different defin's running on two different databases.

What are these locks here to do?

Copy link
Member

Choose a reason for hiding this comment

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

I think he has just locked every method. The key ones are ::create*/replace* methods which will block simultaneous parsing.

@LucasYuki I think some of these do nothing more than set a manipulate local variables and not the db and therefore do not need locks (eg ctor, dtor, skip). Please analyze it a bit more.

delete _reader;
}

void defin::skipConnections()
{
std::lock_guard<std::mutex> lock(_def_mutex);
_reader->skipConnections();
}

void defin::skipWires()
{
std::lock_guard<std::mutex> lock(_def_mutex);
_reader->skipWires();
}

void defin::skipSpecialWires()
{
std::lock_guard<std::mutex> lock(_def_mutex);
_reader->skipSpecialWires();
}

void defin::skipShields()
{
std::lock_guard<std::mutex> lock(_def_mutex);
_reader->skipShields();
}

void defin::skipBlockWires()
{
std::lock_guard<std::mutex> lock(_def_mutex);
_reader->skipBlockWires();
}

void defin::skipFillWires()
{
std::lock_guard<std::mutex> lock(_def_mutex);
_reader->skipFillWires();
}

void defin::continueOnErrors()
{
std::lock_guard<std::mutex> lock(_def_mutex);
_reader->continueOnErrors();
}

void defin::namesAreDBIDs()
{
std::lock_guard<std::mutex> lock(_def_mutex);
_reader->namesAreDBIDs();
}

void defin::setAssemblyMode()
{
std::lock_guard<std::mutex> lock(_def_mutex);
_reader->setAssemblyMode();
}

void defin::useBlockName(const char* name)
{
std::lock_guard<std::mutex> lock(_def_mutex);
_reader->useBlockName(name);
}

dbChip* defin::createChip(std::vector<dbLib*>& libs,
const char* def_file,
dbTech* tech)
{
std::lock_guard<std::mutex> lock(_def_mutex);
return _reader->createChip(libs, def_file, tech);
}

Expand All @@ -109,11 +124,13 @@ dbBlock* defin::createBlock(dbBlock* parent,
const char* def_file,
dbTech* tech)
{
std::lock_guard<std::mutex> lock(_def_mutex);
return _reader->createBlock(parent, libs, def_file, tech);
}

bool defin::replaceWires(dbBlock* block, const char* def_file)
{
std::lock_guard<std::mutex> lock(_def_mutex);
return _reader->replaceWires(block, def_file);
}

Expand Down
Loading
Loading