Skip to content

feat: restore debug build for c part of oneDAL - #3725

Open
Alexandr-Solovev wants to merge 4 commits into
uxlfoundation:mainfrom
Alexandr-Solovev:dev/asolovev_dwarf_addition
Open

feat: restore debug build for c part of oneDAL#3725
Alexandr-Solovev wants to merge 4 commits into
uxlfoundation:mainfrom
Alexandr-Solovev:dev/asolovev_dwarf_addition

Conversation

@Alexandr-Solovev

@Alexandr-Solovev Alexandr-Solovev commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Description

Summary

Strengthens the ABI check job in two ways:

  1. Build host libraries with debug symbols so abidiff can use DWARF and perform full function/variable sub-type diffs (previously it only saw stripped ELF symbols and degraded to add/remove detection on every library).
  2. Add a cross-flavor public-symbol drift check to abi_check.sh that compares the set of newly-exported symbols between the host (_c) and DPC++ flavors of each library and hard-fails when they diverge.

Changes

.github/workflows/ci.yml

  • Make daal and Make onedal (target oneapi_c) now pass --debug symbols to build.sh, so libonedal_core.so, libonedal.so, libonedal_thread.so, and libonedal_parameters.so ship DWARF into the ABI job.
  • oneapi_dpc intentionally stays without debug symbols — DPC++ device DWARF blows up per-object size and icx peak RSS at -j20 and OOMs the runner. A TODO comment documents follow-ups (-Xarch_device -g0 / -fsycl-device-debug=none / larger runner) for enabling it later.
  • The "Check ABI conformance" step banner now spells out which libraries were built with debug info and which fall back to symbol add/remove.

.ci/scripts/abi_check.sh

Adds a supplemental block after the existing per-library abidiff loop:

  • For each (_c, _dpc) library pair
    (libonedal.solibonedal_dpc.so, libonedal_parameters.solibonedal_parameters_dpc.so):
    • Compute the set of newly-exported .dynsym symbols on both sides
      (comm -13 over nm -D --defined-only --extern-only output against __release_lnx_main).
    • Diff the two "new" sets. Any symbol added to one flavor but not the other is reported as ::error:: and increments RETURN_CODE, turning CI red.
  • Works on .dynsym alone — no debug info required, so it complements the DWARF-based abidiff pass on the stripped DPC++ libraries.
  • libonedal_core.so / libonedal_thread.so skipped (no _dpc counterpart).
  • if [ ! -f ] guards make the block a no-op when a flavor is missing.

Rationale

Under the pre-change layout, the DPC++ libraries were built stripped, so abidiff on libonedal_dpc.so silently degraded to symbol-add/remove detection while advertising itself as an ABI check. Any layout change, return-type break, or member-function signature change on a class exported by the DPC++ flavor would pass. The cross-flavor drift check gives us a cheap second line of defense: a PR that adds a public symbol to only one flavor almost always indicates either a missed re-export or an unintentional divergence of the public surface, and a reviewer should look at it either way.

Completeness and readability

  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation to reflect the changes or created a separate PR with updates and provided its number in the description, if necessary.
  • Git commit message contains an appropriate signed-off-by string (see CONTRIBUTING.md for details).
  • I have resolved any merge conflicts that might occur with the base branch.

Testing

  • I have run it locally and tested the changes extensively.
  • All CI jobs are green or I have provided justification why they aren't.
  • I have extended testing suite if new functionality was introduced in this PR.

Performance

  • I have measured performance for affected algorithms using scikit-learn_bench and provided at least a summary table with measured data, if performance change is expected.
  • I have provided justification why performance and/or quality metrics have changed or why changes are not expected.
  • I have extended the benchmarking suite and provided a corresponding scikit-learn_bench PR if new measurable functionality was introduced in this PR.

@Alexandr-Solovev
Alexandr-Solovev marked this pull request as ready for review July 31, 2026 15:13
Copilot AI review requested due to automatic review settings July 31, 2026 15:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR strengthens the Linux ABI conformance job by improving ABI diff fidelity (via DWARF-enabled host builds) and adding an additional guardrail that detects cross-flavor public symbol drift between host (_c) and DPC++ (_dpc) library variants.

Changes:

  • Build host-side libraries with REQDBG=symbols (via --debug symbols) so abidiff can use DWARF for deeper ABI comparisons.
  • Add a cross-flavor “newly-exported symbols” drift check in .ci/scripts/abi_check.sh to fail CI when host vs DPC++ newly-added public symbols diverge.
  • Improve ABI job console messaging to clarify which libraries use DWARF-based checks vs ELF-symbol-only fallback.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
.github/workflows/ci.yml Enables debug-symbol builds for host libraries used by the ABI job and documents why DPC++ remains non-DWARF in CI.
.ci/scripts/abi_check.sh Adds a cross-flavor symbol drift check after abidiff to detect mismatched newly-exported public symbols.

Comment thread .ci/scripts/abi_check.sh
Comment thread .ci/scripts/abi_check.sh Outdated
@Alexandr-Solovev
Alexandr-Solovev force-pushed the dev/asolovev_dwarf_addition branch from d408aa2 to 2f0223d Compare August 3, 2026 06:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants