Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
6 changes: 4 additions & 2 deletions src/odb/src/db/dbWire.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,8 @@ state_machine_update: {
}
} else if (state <= 3) {
if ((opcode & WOP_EXTENSION) && !ignore_ext) {
cur_ext = wire->data_[idx + 1];
const WireOp wire_op = static_cast<WireOp>(opcode & WOP_OPCODE_MASK);
cur_ext = wire_op == kColinear ? wire->data_[idx] : wire->data_[idx + 1];
Comment on lines +559 to +560
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This logic is duplicated in the getSegment overload at lines 790-791. To improve maintainability and avoid code duplication, consider extracting this logic into a static helper function. This aligns with the principle of encapsulating logic in helper functions to avoid duplication.

For example:

static int getCurExt(const _dbWire* wire, unsigned char opcode, int idx)
{
  const WireOp wire_op = static_cast<WireOp>(opcode & WOP_OPCODE_MASK);
  return wire_op == kColinear ? wire->data_[idx] : wire->data_[idx + 1];
}

This would allow you to call cur_ext = getCurExt(wire, opcode, idx); in both locations.

As a smaller change, this logic can be condensed into a single line.

      cur_ext = (static_cast<WireOp>(opcode & WOP_OPCODE_MASK) == kColinear) ? wire->data_[idx] : wire->data_[idx + 1];
References
  1. Encapsulate the logic for calculating master symmetry in a helper function to avoid code duplication.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that conceptually it would be better to refactor the entire state machine as there are two large blocks of code for it. That said, it would be make the review rather painful if included in this PR.

has_cur_ext = true;
}
}
Expand Down Expand Up @@ -786,7 +787,8 @@ state_machine_update: {
}
} else if (state <= 3) {
if ((opcode & WOP_EXTENSION) && !ignore_ext) {
cur_ext = wire->data_[idx + 1];
const WireOp wire_op = static_cast<WireOp>(opcode & WOP_OPCODE_MASK);
cur_ext = wire_op == kColinear ? wire->data_[idx] : wire->data_[idx + 1];
has_cur_ext = true;
}
}
Expand Down
41 changes: 41 additions & 0 deletions src/odb/test/cpp/TestDbWire.cc
Original file line number Diff line number Diff line change
Expand Up @@ -150,4 +150,45 @@ TEST_F(OdbMultiPatternedTest, WireColorIsClearedOnNewPath)
EXPECT_EQ(decoder.getColor().value(), /*mask_color=*/2);
}

// For a wire with two segments connected by a kColinear junction
// with extensions, check if the shape of the second segment is
// correctly computed.
TEST_F(OdbMultiPatternedTest, GetSegmentWithColinearExtension)
{
dbTech* tech = lib_->getTech();
dbTechLayer* metal1 = tech->findLayer("met1");
ASSERT_NE(metal1, nullptr);

dbNet* net = dbNet::create(block_, "net");
dbWire* wire = dbWire::create(net);
dbWireEncoder encoder;
encoder.begin(wire);

// Create segment A.
const int junction_x = 50000;
const int junction_y = 10000;
encoder.newPath(metal1, dbWireType::ROUTED);
encoder.addPoint(0, junction_y);
const int junction_id = encoder.addPoint(junction_x, junction_y);

// Create segment B.
const int extension = 800;
encoder.newPathExt(junction_id, extension); // Inserts kColinear junction.
const int segment_b_length = 500;
const int segment_b_shape_id
= encoder.addPoint(junction_x + segment_b_length, junction_y);
encoder.end();

dbShape shape;
wire->getSegment(segment_b_shape_id, shape);

const int half_width = metal1->getWidth() / 2;
const odb::Rect expected(junction_x - extension,
junction_y - half_width,
junction_x + segment_b_length + half_width,
junction_y + half_width);

EXPECT_EQ(shape.getBox(), expected);
}

} // namespace odb
Loading