Skip to content

fix(miopen): resolve uint64_t typedef conflict with system headers on LP64 Linux - #12623

Merged
schung-amd merged 6 commits into
developfrom
users/schung-amd/fix-miopen-cstdint-lp64-typedef-conflict
Sep 29, 2026
Merged

schung-amd merged 6 commits into
developfrom
users/schung-amd/fix-miopen-cstdint-lp64-typedef-conflict

Conversation

@schung-amd

Copy link
Copy Markdown
Contributor

Motivation

miopen_cstdint.hpp causes a hard compilation error on LP64 Linux (all 64-bit Linux and macOS) when system <stdint.h> is also visible during MIOpen's runtime kernel compilation (comgr/HIPRTC JIT path).

The header typedefs uint64_t as __hip_internal::uint64_t (unsigned long long), but on LP64 platforms <stdint.h> defines uint64_t as unsigned long. These are distinct C++ types despite being the same width (8 bytes), so the compiler rejects the redefinition:

error: typedef redefinition with different types
  ('__hip_internal::uint64_t' (aka 'unsigned long long') vs '__uint64_t' (aka 'unsigned long'))

This affects any GPU target where MIOpen JIT-compiles kernels at runtime (i.e. when pre-compiled kernel databases are missing). It is most commonly hit on consumer GPUs like gfx1032 that lack .fdb.txt entries, but can affect any target under the right conditions. 29 MIOpen kernel files include this header.

Reported in ROCm/TheRock#6258

Technical Details

On LP64 platforms (64-bit Linux, macOS):

  • unsigned long = 64-bit, used by system <stdint.h> for uint64_t
  • unsigned long long = 64-bit, used by __hip_internal::uint64_t
  • These are distinct C++ types despite identical size and alignment

On LLP64 platforms (Windows):

  • unsigned long = 32-bit
  • unsigned long long = 64-bit, matches system uint64_t
  • No conflict exists

The fix uses __LP64__ (defined by GCC/Clang on LP64 platforms) to select the correct underlying type for each platform, matching what the system <stdint.h> provides. This is consistent with the existing WIN32 guard in test/gtest/CMakeLists.txt (line 341) which already passes -U__LP64__ on Windows builds.

Reproducer

Minimal reproduction — compile with any HIP-capable clang:

#include <stdint.h>                         // system: uint64_t = unsigned long
typedef unsigned long long uint64_t;        // miopen: uint64_t = unsigned long long
// error: typedef redefinition with different types

This is what happens when comgr compiles a MIOpen kernel that includes both system headers and miopen_cstdint.hpp. The reporter hit it via the MIOpenNeuron.cpp kernel on gfx1032, but any of the 29 kernel files that include miopen_cstdint.hpp can trigger it.

Other scenarios that can trigger this:

  • Any GPU target missing pre-compiled kernel DB entries (.fdb.txt), forcing JIT
  • Custom MIOpen builds where the kernel DB is not fully populated
  • New GPU targets added before their kernel DBs are generated

Issue Tracking

GitHub issue: ROCm/TheRock#6258

Test Plan

Added test/gtest/miopen_cstdint_lp64.cpp — a compile-time regression test that includes both <cstdint> and miopen_cstdint.hpp with the MIOPEN_HIP_RUNTIME_COMPILE path active, and verifies via std::is_same that the resulting uint64_t/int64_t types match the system definitions. The test is auto-discovered by the existing file(GLOB TESTS *.cpp) in the gtest CMakeLists.txt.

Test Result

Verified locally on Ubuntu 24.04 (LP64), ROCm 10.0 (amdclang 23.0.0git), gfx1100:

  • Before fix: typedef redefinition with different types error
  • After fix: compiles cleanly, types match system <stdint.h>

🤖 Generated with Claude Code

… LP64 Linux

miopen_cstdint.hpp typedefs uint64_t as __hip_internal::uint64_t
(unsigned long long), but on LP64 Linux <stdint.h> defines uint64_t as
unsigned long. These are distinct C++ types despite being the same width,
causing a hard compilation error when both definitions are visible during
MIOpen's runtime kernel compilation (comgr/HIPRTC JIT path).

Use platform-appropriate types: unsigned long on LP64 (Linux/macOS) and
unsigned long long on LLP64 (Windows), matching what the system <stdint.h>
provides on each platform.

Fixes: ROCm/TheRock#6258

Co-Authored-By: Claude <noreply@anthropic.com>
@schung-amd
schung-amd requested a review from a team as a code owner September 26, 2026 04:03
@therock-pr-bot

therock-pr-bot Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

✅ All Checks Passed — Ready for Review

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🔎 pre-commit ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —
🤖 therock-pr-bot ✅ Pass —

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@therock-pr-bot

Copy link
Copy Markdown

Pre-commit check failed

⛔ pre-commit failed

Please run locally:

  • python -m pip install pre-commit
  • pre-commit install
  • pre-commit run --all-files --show-diff-on-failure

This repo uses .pre-commit-config.yaml.

schung-amd and others added 2 commits September 26, 2026 09:37
Rename test suites to CPU_MiopenCstdintLP64_NONE to match
the required pattern: (CPU|GPU)_<Name>_<Datatype>.

Co-Authored-By: Claude <noreply@anthropic.com>
@therock-pr-bot

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

@averinevg averinevg 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.

Thanks for the fix. Left some comments.

schung-amd and others added 3 commits September 28, 2026 10:02
- Use __UINT64_TYPE__ / __INT64_TYPE__ compiler builtins instead of
  __LP64__ guards (reviewer suggestion)
- Switch test to SPDX license header
- Rewrite test to exercise the real comgr JIT path via Handle::AddKernel
  with inline kernel source including miopen_cstdint.hpp

Co-Authored-By: Claude <noreply@anthropic.com>
…flict

The original bug requires both system <stdint.h> and miopen_cstdint.hpp
to be visible during JIT compilation. Include <stdint.h> explicitly in
the test kernel source to ensure the typedef conflict would be triggered
with the old code, matching the MIOpenNeuron.cpp failure pattern.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>

@averinevg averinevg 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.

LGTM

@schung-amd
schung-amd merged commit d91c43b into develop Sep 29, 2026
119 checks passed
@schung-amd
schung-amd deleted the users/schung-amd/fix-miopen-cstdint-lp64-typedef-conflict branch September 29, 2026 14:36
@assistant-librarian

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

mattwill-amd pushed a commit that referenced this pull request Oct 14, 2026
JIRA ID: ALMIOPEN-2913

## Motivation

Only 5 of the 30 commits that landed against `projects/miopen` since the
3.6.2 version bump (`2d20a8f775e`, #12211) were reflected in
`CHANGELOG.md`. The remaining 25 were reviewed and the 15
externally-visible ones documented, so the ROCm 10.2.0 section
accurately reflects what shipped.

14 entries added (two of the fixes share one entry), plus a
duplicate-heading fix.

### Added
* [Conv] NHWC (channel-last) support in the GEMM convolution solvers,
with new im2col/col2im kernels (#8693)
* [Conv] Cross-architecture AI heuristics for immediate mode
(`MIOPEN_FIND_MODE=2`), extending data-driven selection to Navi (#9680)
* [Conv] Missing 3x3 stride-1 pad-1 tile-28 CK depthwise backward-data
instance — ALMIOPEN-2423 (#11271)

### Changed
* [Conv] `ConvHipConv` held out of the AI perf-config picker until its
model is retrained — ALMIOPEN-2494 (#13337)
* [Conv] CK byte-span gate lifted for forward and backward-data, now
that CK fixes the 32-bit offset overflow itself — ALMIOPEN-2645 (#12396)

### Optimized
* [Conv] `ConvDirectNaiveConv`: per-direction kernel files, float
accumulators, spatial tiling, bfloat16 path — ALMIOPEN-2485/2486 (#7529)
* Kernel cache reuses cached SQLite connections instead of reopening per
load/store — ALMIOPEN-2598 (#11933)

### Resolved Issues
* [Conv] Incorrect backward-weights results from the gfx950 direct
kernel at `waves_q == 1` (missing cross-lane ordering on LDS staging
reuse) — ROCM-31508 (#12406)
* [Conv] Up to 8x slower hipconv `direct`/`direct_l1`/`direct_wgrad`
under ROCm 10.1's full AMDGCN LTO default; fixed with `-fno-offload-lto`
— ROCM-31334 (#12328)
* [Conv] gfx942 multi-node training throughput regression from backward
GEMM on small-batch 1x1 NHWC shapes — ROCM-31155 (#13232)
* [Conv] Invalid CK perf-config entries in the gfx1201 and gfx1100
system databases — ROCM-29026 (#12088, #12117)
* [BatchNorm] JIT compilation failures on non-GFX9 targets; Welford DPP
assembly restricted to GFX9 — ALMIOPEN-2838 (#12889)
* Out-of-bounds write in `miopenLoadSolution` when a serialized solution
carries more than three work dimensions — ROCM-31082 (#12242)
* `uint64_t` typedef conflict between `miopen_cstdint.hpp` and system
`<stdint.h>` on LP64 Linux (#12623)

## Technical Details

Single-file change to `projects/miopen/CHANGELOG.md`.

The 10.2.0 section had **two** `### Resolved Issues` headings. The two
entries under the trailing one (`ConvHipConv` gfx1250 crash/LDS race,
and the perf-db reload config loss) were merged into the first, and the
trailing heading removed — this accounts for the 2 removed bullets in
the diff. Section order is now `Resolved Issues → Added → Changed →
Optimized → Removed`, matching the 3.6.1 section. `Optimized` is new to
this release section.

Already documented, so untouched: #12874 (reduce int32 rejection),
#12000 / #13016 / #13213 (hipconv v0.3.1 / v0.3.2 / v0.4.0), #11761
(gfx803 removal).

## Notes

Deliberately excluded as not externally visible (10 commits):

* **hipDNN convolution forwarding (#12276)** —
`MIOPEN_ENABLE_HIPDNN_WRAPPER` defaults to `OFF`
(`projects/miopen/CMakeLists.txt:162`), so no shipped build compiles the
forwarding code and the runtime `MIOPEN_HIPDNN_FORWARDING` flag has
nothing to enable.
* **Test-only**: #13289 (gfx1250 depthwise test applicability,
ALMIOPEN-2841), #11610 (hipDNN parity harness), #12660 (Jenkins status
post).
* **Internal kernel-math cleanup, generated ISA unchanged**: #13312
(drop direct `__ocml` calls), #13190 (remove broken single-argument
`__half` overloads).
* **Add/revert pairs, net no change**: #12466 + #13066 (gfx1101 Navi
system DBs), #12496 + #12800 (gfx1250 in the dev image arch lists).

## Risk Assessment

Documentation-only change to a single Markdown file; no code, build, or
runtime impact.

**Risk Level: 🟢 Low**

## Test Plan

None required — Markdown-only. Verified that the entry topics are not
already documented elsewhere in the file, that no content was lost when
merging the duplicate heading, and that every new entry maps to a commit
in `2d20a8f775e..origin/develop -- projects/miopen`.

## Test Result

16 bullets added, 2 relocated — 14 net-new entries covering 15 commits.
Duplicate heading removed; section ordering matches 3.6.1.

Co-authored-by: Claude <noreply@anthropic.com>
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