gpl, pdn: fix NaN clamp bypass and -min_width_layers edge cases#10735
gpl, pdn: fix NaN clamp bypass and -min_width_layers edge cases#10735ranaumarnadeem wants to merge 1 commit into
Conversation
Signed-off-by: ranaumarnadeemgit config --global user.name ranaumarnadeem <ranaumarnadeem632@gmail.com>
There was a problem hiding this comment.
Code Review
This pull request introduces NaN handling for the skew_fraction parameter in replace.cpp, defaulting to 0.10f when NaN is encountered. In connect.cpp, it adds validation to ensure that layers specified in setMinWidthLayers are intermediate routing layers, and updates generateMinEnclosureViaRects to snap coordinates to the manufacturing grid. The reviewer suggests replacing the hardcoded magic number 0.10f with a named constant to improve maintainability.
| skew_fraction = std::isnan(skew_fraction) | ||
| ? 0.10f | ||
| : std::clamp(skew_fraction, 0.0f, 1.0f); |
There was a problem hiding this comment.
The default skew fraction value 0.10f is hardcoded as a magic number. To improve maintainability and adhere to the general rule of defining tunable parameters or default values as named constants, please define a named constant for this default value.
constexpr float default_skew_fraction = 0.10f;
skew_fraction = std::isnan(skew_fraction)
? default_skew_fraction
: std::clamp(skew_fraction, 0.0f, 1.0f);References
- Define tunable parameters as named constants instead of using hardcoded magic numbers.
|
duplicate of #10734 |
Summary
Two bugs in the
-min_width_layersfeature added by #10689:1. Off-grid via metal when
-min_width_layersis activegenerateMinEnclosureViaRectscomputed min-width via rect edges ascenter ± min_width/2with no manufacturing-grid snap. When a layer islisted in
-min_width_layersthe min-width rect becomes the sole viarect (the full-overlap rect is dropped via
std::move), so unsnappededges produced off-grid metal. Fixed by snapping with
TechLayer::snapToManufacturingGrid, matching the existing pattern ingenerateComplexStackedViaRects.2. Invalid
-min_width_layerslayer silently ignoredsetMinWidthLayersinserted any layer unconditionally. A layer that isnot an intermediate routing layer of the connect pair (wrong routing
level, cut layer) never matches inside
generateMinEnclosureViaRects,so the user's constraint had no effect with no diagnostic. Fixed by
validating each entry against
intermediate_routing_layers_; invalidlayers now emit
[WARNING PDN-0501]and are skipped.Type of Change
Impact
-min_width_layersis now guaranteed to landon the manufacturing grid. No change to behavior for connects that do
not use
-min_width_layers.-min_width_layersarguments (e.g. a cut layer or alayer outside the connect pair's stack) now produce a
[WARNING PDN-0501]instead of being silently ignored.Verification
./etc/Build.sh).Related Issues
Introduced by #10689 (pdn: bridge stacked vias to on-grid neighbors and
add -min_width_layers).