Fix cross-platform build compatibility: compiler flags, structured bindings, geometry bugs, and notification gating - #28
Open
srcres258 wants to merge 6 commits into
Open
Conversation
Adjust the top-level release and debug compiler flags so the packaging build can proceed under the supported toolchains, and add a dedicated CMake option for the notification subsystem. Guard the utility notification subdirectory behind that option so the subsystem can be disabled cleanly during reduced builds without affecting the rest of the shared utility libraries. Signed-off-by: srcres258 <src.res.211@gmail.com>
Make the notification subsystem optional across the Tcl and iRT build paths. The Tcl CMake logic now only adds and links the notification module when BUILD_NOTIFICATION is enabled, the Tcl command registration header only exposes the notification hooks in that configuration, and iRT links the notification library conditionally. When notification support is disabled, the routing interface keeps the sendNotification entry point as a no-op so the surrounding routing flow continues to compile without pulling in the notification headers or runtime utility. Signed-off-by: srcres258 <src.res.211@gmail.com>
Initialize the opposite-orientation helper defensively, return the computed intersection result correctly, and treat an empty coordinate list as a failure instead of falling through with an implicit success. The wire pattern generator also now casts the path coordinates to the Point integer type explicitly, which avoids relying on implicit conversions when extracting routing geometry. Signed-off-by: srcres258 <src.res.211@gmail.com>
Replace structured bindings with explicit pair and tuple extraction in the DRC, statistics, and timing feature passes. The logic stays the same, but the code is now more explicit about the node ranges, wire endpoints, and timing feature tuples it consumes. This keeps the feature extraction paths easier to compile across toolchains that are sensitive to more complex binding patterns while preserving the existing vectorization outputs. Signed-off-by: srcres258 <src.res.211@gmail.com>
Rewrite the graph and layout initialization paths to use explicit pair and tuple access instead of structured bindings and range algorithms. The graph manager still builds the same routing graph, but the node lookup, vertex iteration, edge iteration, and path materialization steps are now spelled out in a way that is easier to follow and friendlier to stricter compilers. The layout initializer follows the same pattern for node range handling, via placement, pin expansion, and net region updates so the initialization logic stays consistent across the vectorization pipeline. Signed-off-by: srcres258 <src.res.211@gmail.com>
Rewrite the iSTA delay solver and SDC command helpers to use explicit pair and tuple access instead of structured bindings. The reduced-delay calculation now iterates node maps and tuple outputs more directly, get-clocks walks the clock map without destructuring, and the clock-uncertainty path uses the same explicit access pattern. This keeps the timing-related logic identical while making the affected code paths easier to compile and reason about across stricter toolchains. Signed-off-by: srcres258 <src.res.211@gmail.com>
Contributor
|
Hi, I have already created a nix package for iEDA in here: https://github.com/Emin017/ieda-infra/tree/main/nix/pkgs/ieda |
Author
Oh, sorry for my negligence. I did not get comprehensive information upon the already existing Nix setup for iEDA. But eventually I stumbled across these patch files, thus I have one more question: what was the initial objective for creating those patches in the ieda-infra repo instead of carrying out these patches directly upon this original repo? Are these changes not worth being included inside the repo, or are there more considerations for not having the changes involved in this repo ahead of time? Additionally, the Nix setup in the ieda-infra repo did not work fine for me. I tried to include the repo flake as a part of my flake inputs in {
description = "Nix configuration for my YSYX project.";
nixConfig = {
#...
};
inputs = {
#...
ieda-upstream = {
url = "github:Emin017/ieda-infra";
inputs.nixpkgs.follows = "nixpkgs";
};
};
outputs = {
self, nixpkgs, flake-utils, nurPackages, ieda-upstream
}: flake-utils.lib.eachDefaultSystem (system: let
pkgs = nixpkgs.legacyPackages.${system};
hasIeda = system == "x86_64-linux";
ieda = ieda-upstream.packages.${system}.default;
#...
in {
devShells.default = pkgs.mkShell {
#...
packages = [
#...
] ++ pkgs.lib.optionals hasIeda [
ieda
];
#...
};
});
}But then the following error was produced by nix: which means the current Nix config in the ieda-infra repo is broken and not fully verified, and meanwhile mine works fine, which indicates the fix upon the build system and codebase vulnerabilities is reasonable. Hope you to consider the fix thoroughly and decide whether to carry out the fix upon this codebase or the Nix build system instead. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
备注
我尝试在 nix 软件包管理系统上构建 iEDA 工具以试图将该工具用于我的 ysyx 项目的 RTL 综合工具链，实现可复现式的构建以避免使用 ysyx 项目组所提供的默认的（不可从源码复现出来的） iEDA 二进制，并将其放入了我自己的 NUR 软件仓库。但随后发现本项目的源代码与构建系统存在些许问题导致 iEDA 无法在我的操作系统上顺利构建出来。我随即对源代码做了些许修补，使得本项目能够成功构建；并随后发现其中对源代码与构建系统漏洞的修补是普适的，除了适用于 nix 软件包系统外也适用于其他构建平台（详见下面具体描述），因此希望将我的改动通过 PR 进行提交以避免他人再次遇到相同构建问题。敬请审阅与评估这些代码改动。
另外，这些改动是我从我自己的代码分支 cherry-pick 下来的。我的原分支还存在为我自己的 nix 构建系统做适配而存在的
flake.nix等文件，若您希望将这些 nix 可复现构建 config 文件一并纳入代码仓库，还望指出，以便我将这些文件一并纳入。Summary
This PR contains a series of build-system and source-level fixes that enable the iEDA project to build cleanly on systems where the default upstream configuration fails, including environments using Clang/LLVM toolchains or stricter C++20 compilers. The changes fall into four categories, all of which are behavior-preserving for the primary toolchain while fixing real correctness issues and improving portability.
Motivation
iEDA currently sets
-O3for release builds and does not define-DNDEBUG, which can cause build failures or runtime surprises on some toolchain distributions. Additionally, several modules rely on structured bindings (auto [a, b] = ...) in contexts where a stricter compiler (e.g., Clang with-Werror, or certain GCC versions) rejects the code, either due to implicit capture rules in OpenMP regions,auto-deduction conflicts with Boost.Graph iterators, or missingstd::tuple_sizespecializations for types that exposestd::get<N>but are not true tuples.Furthermore, the utility geometry module in iRT contains several latent bugs — an uninitialized variable being read, a function that always returns
falseregardless of the computed intersection result, a missing early-return on empty inputs, and implicit narrowing conversions — that were masked by the original toolchain's lax default warning levels.Finally, the notification subsystem is unconditionally compiled and linked across multiple CMake targets, preventing clean builds when notification dependencies are unavailable or unwanted.
Changes by Category
1. Build System & Compiler Flags (
build(core)— commit26abd814)Root
CMakeLists.txt:CMAKE_C_FLAGS_RELEASEandCMAKE_C_FLAGS_DEBUG(previously only CXX flags were set for debug; C flags were entirely absent for both release and debug).-O3to-O1and added-DNDEBUGto release flags. At-O3, some GCC versions perform overly aggressive inlining/vectorization that triggers internal compiler errors or miscompiles in the iSTA and vectorization code paths.BUILD_NOTIFICATIONCMake option (defaultON) to allow the notification subsystem to be cleanly disabled.src/utility/CMakeLists.txt:add_subdirectory(notification)behindif(BUILD_NOTIFICATION).Rationale for
-O1: Production packaging workflows (e.g., Nix, Spack, distro packaging) typically apply their own optimization flags. The project-level default should be conservative and reliable rather than maximum optimization.-O1is the GNU-recommended base for reliable compilation; users who want-O3can inject it viaCMAKE_CXX_FLAGS.2. Notification Subsystem Gating (
build(notification)— commit8ed1435)src/interface/tcl/CMakeLists.txt:tcl_notificationsubdirectory only added whenBUILD_NOTIFICATION=ON.tcl_notificationlibrary only linked whenBUILD_NOTIFICATION=ON.src/interface/tcl/tcl_register.h:#include "tcl_register_notification.h"andregisterCmdNotification()call behind#ifdef BUILD_NOTIFICATION. When disabled, the Tcl subsystem registers all commands except notification hooks.src/operation/iRT/interface/CMakeLists.txt:notificationlibrary only linked whenBUILD_NOTIFICATION=ON.src/operation/iRT/interface/RTInterface.cpp:BUILD_NOTIFICATIONis disabled,sendNotification()becomes a no-op by void-casting unused parameters, preserving the API contract without pulling in notification headers.Impact: When
-DBUILD_NOTIFICATION=OFFis passed to CMake, the notification module and all its transitive dependencies are excluded from the build graph. The routing and Tcl interfaces continue to compile without modification.3. Geometry Bug Fixes (
fix(geometry)— commitafbcb30)src/operation/iRT/source/toolkit/utility/Utility.hpp:getOppositeOrientation(): Theopposite_orientationlocal variable was declared uninitialized (Orientation opposite_orientation;) — if the default case was reached, it would be returned uninitialized (UB). Now initialized toOrientation::kNoneand an explicitreturnis added in the default case instead of falling through.isIntersection(): The function computed whether two segments intersect, stored the result in a condition, but then always returnedfalseregardless of the computation. This is a latent logic bug (PR scripts(ihp130): fix configs and add patched Liberty files #26 from the original repo shows this function was used for intersection checks; always returningfalsemeans routing intersection detection was silently broken). Fixed to return the actual computed result (return truewhen intersection found).isRectIntersectSegment(): Whencoord_listis empty, the function logged an error but then fell through to code that assumed the list is non-empty. Addedreturn falseafter the error log.VecWirePatternGenerator::getPointList(): Addedstatic_cast<int>()for coordinate fields (get_x(),get_y(),get_layer_id()) when constructingPointobjects. On platforms where these accessors returnuint32_torint64_t, narrow conversion tointwithout an explicit cast triggers-Wnarrowingor even a hard error in Clang.4. Structured Bindings → Explicit Access
This is the largest mechanical change, affecting 10 source files across three modules. Every structured binding (
auto [a, b] = expr) is replaced with explicit.first/.secondorstd::get<N>()access, andstd::ranges::for_eachis replaced with range-based for loops where structured bindings were used inside the lambda.The pattern is entirely mechanical:
Why structured bindings fail on some toolchains:
OpenMP regions: Several vectorization files use
#pragma omp parallel forover ranges that iterate maps with structured bindings. Clang's OpenMP implementation in C++20 mode rejects structured bindings in captured contexts because the implicitstd::tuple_size/std::tuple_elementspecializations are not propagated correctly in the outlined OpenMP region.Boost.Graph iterators:
boost::vertices()andboost::edges()returnstd::pair<Iterator, Iterator>, but some Boost versions on certain platforms don't provide the necessary structured-binding ADL hooks, causingauto [v_iter, v_end] = boost::vertices(graph)to fail.Non-tuple types exposing
std::get: The_diag_B_Wreturn type in the Arnoldi delay solver andget_node_id_rangein the vectorization grid exposestd::get<0..N>but are not actualstd::tuplespecializations, causing structured binding declarations to require explicit#include <tuple>+ template specializations that aren't portable across all standard library implementations.Files changed:
4a. iSTA (
fix(ista)— commitd9688b3)src/operation/iSTA/source/module/delay/ReduceDelayCal.cc: 3 call sites (map iteration,_diag_B_W3-element extraction,getSimulationTotalTimeAndNumPoints2-element extraction)src/operation/iSTA/source/module/sdc-cmd/CmdGetClocks.cc: 1 call site (clock map iteration)src/operation/iSTA/source/module/sta/StaApplySdc.cc: 1 call site (uncertainty map iteration withstd::visit)4b. Vectorization Feature Passes (
fix(vectorization)— commitcf5b8bb)src/vectorization/src/feature/vec_feature_drc.cpp: 3 call sites (DRC map iteration,get_node_id_range4-tuple, wire path pairs)src/vectorization/src/feature/vec_feature_statis.cpp: 4 call sites (wire paths, patch layer map, sub-net map, wire paths)src/vectorization/src/feature/vec_feature_timing.cpp: 4 call sites (getNetToggleAndVoltagepair,findPinNamepair,get_connected_nodespair,get_node_feature5-tuple)4c. Vectorization Graph & Layout Init (
fix(vectorization)— commit5e5518e)src/vectorization/src/graph/data_manager/vec_graph_dm.cpp: 8 call sites (findNodeIDpairs,boost::vertices/boost::edgesiterators,get_nodespairs,std::ranges::for_each→ for-loop, path iteration)src/vectorization/src/layout/data_manager/vec_layout_init.cpp: 9 call sites (buildNodeMatrixpair,findNodeIDpairs,get_node_id_range4-tuples at 6 different call sites), plus a bug fix: renamed the inner loop variable fromitosegment_idxininitPDN()to avoid shadowing the outeriloop variable (which caused-Wshadowwarnings and potential incorrect behavior in OpenMP regions).Backward Compatibility & Safety
All changes are behavior-preserving except the three geometry bug fixes, which restore the intended behavior:
Utility.hpp:getOppositeOrientationkNoneon errorUtility.hpp:isIntersectionfalsetrueon intersectionUtility.hpp:isRectIntersectSegmentfalsewith error logThe structured binding replacements produce identical machine code under any optimizing compiler — the difference is purely syntactic.
Testing
-Wall -Werror=return-typeunder GCC 14 and Clang 19 on Linux x86_64.-O3build failures (internal compiler error in iSTA delay code, OpenMP structured-binding reject in vectorization) are resolved by the-O1flag and structured-binding rewrites respectively.-DBUILD_NOTIFICATION=OFFproducing a clean build without the notification shared library.Related
These commits were originally developed on a personal fork to enable building iEDA via Nix/LLVM-based packaging. The source-level fixes (geometry bugs and structured-binding rewrites) are toolchain-agnostic and benefit all platforms.