Skip to content

Fix libomp ABI bug - #1593

Merged
ramakrishnap-nv merged 14 commits into
NVIDIA:mainfrom
aliceb-nv:libomp-ifcopy-fix
Jul 22, 2026
Merged

Fix libomp ABI bug#1593
ramakrishnap-nv merged 14 commits into
NVIDIA:mainfrom
aliceb-nv:libomp-ifcopy-fix

Conversation

@aliceb-nv

@aliceb-nv aliceb-nv commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

This PR adds a workaround resolving a buggy interaction between GCC's generated OMP helpers and LLVM's OpenMP runtime.
This was introduced as a latent issue in the Recursive RINS PR, and then exposed by the fast MPS PR which moved the wheel's runtime to LLVM OpenMP.

In more details:

The failure was caused by an LLVM libomp bug in its GCC GOMP_task compatibility layer: for an included OpenMP task (if(false)), libomp skipped GCC’s generated
firstprivate copy function, leaving a captured non-trivial C++ object unconstructed; the RINS sub-MIP path then read this invalid object and eventually threw
std::bad_array_new_length. The unrelated merge merely exposed the latent runtime-dependent bug in libomp-linked wheels; explicitly executing the included-task
path synchronously avoids the faulty compatibility path.

I will file a min repro and bug to LLVM later.

closes #1592

Description

Issue

Checklist

  • I am familiar with the Contributing Guidelines.
  • Testing
    • New or existing tests cover these changes
    • Added tests
    • Created an issue to follow-up
    • NA
  • Documentation
    • The documentation is up to date with these changes
    • Added new documentation
    • NA

@aliceb-nv aliceb-nv added this to the 26.08 milestone Jul 21, 2026
@aliceb-nv
aliceb-nv requested review from a team as code owners July 21, 2026 11:30
@aliceb-nv aliceb-nv added bug Something isn't working non-breaking Introduces a non-breaking change labels Jul 21, 2026
@aliceb-nv
aliceb-nv requested review from akifcorduk and rg20 July 21, 2026 11:30
@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 148614d1-61b4-4adc-baf8-2ca21a525777

📥 Commits

Reviewing files that changed from the base of the PR and between c21f7a8 and 67c5df3.

📒 Files selected for processing (1)
  • cpp/src/io/mps_parser.cpp

📝 Walkthrough

Walkthrough

Changes

Release alignment and package references

Layer / File(s) Summary
Release workflow and package alignment
.github/workflows/*, RAPIDS_BRANCH, README.md
Reusable workflows and the RAPIDS branch now use release/26.08; installation examples reference 26.08 package versions.

Fast MPS parsing and runtime behavior

Layer / File(s) Summary
Quadratic MPS canonicalization
cpp/src/io/experimental_mps_fast/fast_parser.cpp, cpp/src/io/mps_parser.cpp, cpp/tests/linear_programming/experimental_mps_fast/fast_parser_edge_test.cpp
Quadratic COO entries are validated and canonicalized, int64 specializations are instantiated, and the parser fixture includes an additional off-diagonal record.
RINS scheduling and regression coverage
cpp/src/branch_and_bound/branch_and_bound.cpp, skills/cuopt-developer/SKILL.md, python/cuopt/cuopt/tests/linear_programming/*
RINS runs synchronously inside sub-MIPs and through an OpenMP task otherwise; related guidance and previously skipped tests are updated.
RMM type availability
cpp/src/pdlp/utilities/problem_checking.cuh
The header directly includes the RMM device_uvector definition.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Suggested reviewers: rg20

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning Several workflow, README, skill-doc, and parser/test-fixture edits are unrelated to the libomp workaround. Split the unrelated release/version and parser changes into separate PRs so this change stays focused on the libomp fix and affected tests.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title concisely describes the main change: fixing the libomp ABI bug.
Description check ✅ Passed The description is directly about the libomp/OpenMP workaround and the swath1 crash it fixes.
Linked Issues check ✅ Passed The PR addresses #1592 by fixing the wheel crash and re-enabling the affected swath1 tests.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Jul 21, 2026

Copy link
Copy Markdown

CI Test Summary

8 failed · 23 passed · 0 skipped

@aliceb-nv

Copy link
Copy Markdown
Contributor Author

test failures unrelated. addressed in #1594

@aliceb-nv

Copy link
Copy Markdown
Contributor Author

/merge

ramakrishnap-nv and others added 13 commits July 21, 2026 13:49
RAPIDS main moved to 26.10 after burndown; local builds were pulling
rapids-cmake from RAPIDS main (26.10) while the conda env has 26.08
packages, breaking the build.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
rapids-cmake uses release/X.Y branch naming, not branch-X.Y.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
rapidsai/shared-workflows main moved to 26.10, causing wheel builds to
use 26.10 CI containers. Pin to release/26.08 to match the RAPIDS_BRANCH.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
RMM defines device_uvector inside namespace RMM_NAMESPACE, which expands
to `__attribute__((visibility("default"))) rmm` on GCC/Linux. A forward
declaration in plain `namespace rmm {}` creates a second, distinct class
visible to NVCC, causing '"rmm::device_uvector" is ambiguous' errors.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
…ith include"

This reverts commit c9ae05a.

Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
rmm 26.08.00a61 introduced inline namespace ABI versioning: device_uvector
now lives in rmm::_RMM_26_8 (exposed as rmm::device_uvector via inline
namespace). A plain-namespace forward declaration creates a second, distinct
rmm::device_uvector, causing '"rmm::device_uvector" is ambiguous' errors
in NVCC. Replace with the full header include.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Ramakrishna Prabhu <ramakrishnap@nvidia.com>
@aliceb-nv
aliceb-nv force-pushed the libomp-ifcopy-fix branch from 84a6071 to c21f7a8 Compare July 22, 2026 07:17
@aliceb-nv
aliceb-nv requested a review from a team as a code owner July 22, 2026 07:17
@aliceb-nv
aliceb-nv requested a review from bdice July 22, 2026 07:17
@copy-pr-bot

copy-pr-bot Bot commented Jul 22, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@aliceb-nv
aliceb-nv removed the request for review from bdice July 22, 2026 07:18
@aliceb-nv

Copy link
Copy Markdown
Contributor Author

/ok to test c21f7a8

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cpp/src/io/mps_parser.cpp`:
- Around line 1720-1725: Instantiate check_symmetric_offdiagonal_pairs for
int64_t with both float and double value types alongside the existing
canonicalize_coo_matrix<int64_t, ...> explicit instantiations. Ensure these
emitted symbols satisfy the calls from parse_mps_fast_file<int64_t,
float/double> in the fast-parser translation unit.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e5b06e46-be59-40c0-bb10-c11ddf171c90

📥 Commits

Reviewing files that changed from the base of the PR and between 84a6071 and c21f7a8.

📒 Files selected for processing (15)
  • .github/workflows/build.yaml
  • .github/workflows/multi_gpu_cpp_test.yaml
  • .github/workflows/pr.yaml
  • .github/workflows/test.yaml
  • .github/workflows/trigger-breaking-change-alert.yaml
  • RAPIDS_BRANCH
  • README.md
  • cpp/src/branch_and_bound/branch_and_bound.cpp
  • cpp/src/io/experimental_mps_fast/fast_parser.cpp
  • cpp/src/io/mps_parser.cpp
  • cpp/src/pdlp/utilities/problem_checking.cuh
  • cpp/tests/linear_programming/experimental_mps_fast/fast_parser_edge_test.cpp
  • python/cuopt/cuopt/tests/linear_programming/test_incumbent_callbacks.py
  • python/cuopt/cuopt/tests/linear_programming/test_lp_solver.py
  • skills/cuopt-developer/SKILL.md
💤 Files with no reviewable changes (1)
  • python/cuopt/cuopt/tests/linear_programming/test_lp_solver.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • cpp/src/branch_and_bound/branch_and_bound.cpp
  • python/cuopt/cuopt/tests/linear_programming/test_incumbent_callbacks.py

Comment thread cpp/src/io/mps_parser.cpp
@aliceb-nv

Copy link
Copy Markdown
Contributor Author

/ok to test 67c5df3

@ramakrishnap-nv
ramakrishnap-nv merged commit 87b32af into NVIDIA:main Jul 22, 2026
241 of 268 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working non-breaking Introduces a non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

wheel: swath1.mps aborts with std::bad_array_new_length after fast MPS parser (#1429)

5 participants