Skip to content

fix(clr): report the full unified memory pool on large-memory APUs - #10974

Merged
jaydeeppatel1111 merged 3 commits into
developfrom
users/jaypatel/clr-pal-uma-large-memory
Sep 4, 2026
Merged

jaydeeppatel1111 merged 3 commits into
developfrom
users/jaypatel/clr-pal-uma-large-memory

Conversation

@jaydeeppatel1111

@jaydeeppatel1111 jaydeeppatel1111 commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

PAL under-reports memory on large-memory APUs. It discounts the GART aperture by 25%, and clamps the largest single allocation to a hardcoded 64 GiB regardless of how much memory the part actually has.

Motivation

The 64 GiB maximum is a constant, not a measurement. PAL reports the same 64.00 GiB on every configuration tested, across carve-outs from 8 GB to 160 GB. The clamp's comment still describes a 4 GB blit-kernel limit it stopped matching long ago.

The largest allocation is derived from the largest single heap. On a unified memory part that is the wrong quantity. The carve-out and the aperture are the same DRAM, and a single allocation is not confined to one heap — measured, see below. Deriving the ceiling from one heap leaves the rest of the pool unreachable, and does so by a different amount depending on whether the KMD splits the carve-out.

The aperture discount has no recoverable rationale. The aperture is host DRAM the GPU addresses directly. Discounting it by 25% understates the pool, and because free memory is derived from the same total, it is also what makes hipMemGetInfo saturate.

Technical Details

Three changes, all in PAL:

  • Credit the aperture as gart - min(gart/4, 4G), replacing the 50/75% heuristic. This holds back a quarter of the aperture up to a 4 GiB cap, so the OS is not starved on configurations where the aperture is most of the pool.
  • Derive maxMemAllocSize_ from the whole pool rather than the largest single heap.
  • Raise maxAllocSize_ from 64 to 256 GiB, keeping the clamp in place for every device rather than adding an exception. Without this the previous change is inert, since the clamp would truncate the pool-derived value straight back to 64 GiB.

Issue Tracking

JIRA ID: ROCM-24637

Test Plan

Dry run the hipMalloc guard on below configs
SH 128 : 96+16+16, with split ON/OFF
SH 128 : 8+96+24
GH : 160+32
GH : 8+184

Test Result

hipMalloc allows allocation over gart on APUs.

Direct-map carve-out

The Gmx_UseFbOffsetAsPaOnApu regkey decides whether the carve-out arrives as one segment or is split by BAR round-down. Its default enables direct-map only when the BAR and carve-out sizes differ, the KMD document leaves per-milestone enablement unconfirmed, and that document's own test plan exercises the disabled path — so PAL will meet both states in the field. Both were tested here.

Submission Checklist

@therock-pr-bot

therock-pr-bot Bot commented Aug 31, 2026 •

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/clr/rocclr/device/pal/paldevice.cpp, projects/clr/rocclr/device/pal/palsettings.cpp; no test file found
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

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

🙋 Wish to Override Policy?

@jaydeeppatel1111
jaydeeppatel1111 requested review from b-shi and removed request for b-shi August 31, 2026 17:09
PAL credits only 75% of the GART aperture and clamps maxMemAllocSize_ to 64 GiB.
Where the aperture is host DRAM the GPU addresses directly, that under-reports both
the total pool and the largest possible allocation, and hipMemGetInfo saturates at
zero free once usage passes the understated total. Callers sizing buffers from the
reported maximum self-limit well below what the device delivers.

Credit the aperture in full, holding back a floor of host memory so the OS is not
starved, and derive maxMemAllocSize_ from the whole pool rather than the largest
single heap. One allocation is not confined to a single heap: with the carve-out
split into 64 GiB visible plus 32 GiB invisible, a 100 GiB buffer was written and
verified, which needs both framebuffer heaps in full plus part of the aperture.

The hipMalloc guard summed maxMemAllocSize_ and maxPhysicalMemAllocSize_. On a
unified memory device the aperture is host DRAM already counted in the first term,
so the sum double-counts it and lands near twice the memory present. Bound on
maxMemAllocSize_ alone, as hipMallocPitch and the mempool allocator already do.

All of it is gated on a new per-device umaLargeMemory_ setting, currently set only
for the parts where large unified memory has been validated, so every other device
and the whole ROCr path keep their existing behavior.

On a 128 GB Strix Halo with a 96 GiB carve-out this moves the reported pool from
107.87 to 111.82 GiB and the largest allocation from 64.00 to 111.82 GiB, with
allocations verified byte for byte to 110 GiB in both direct-map states.

Addresses ROCM-24637.

Co-authored-by: Cursor <cursoragent@cursor.com>
@jaydeeppatel1111
jaydeeppatel1111 force-pushed the users/jaypatel/clr-pal-uma-large-memory branch from 9518b0a to 1ced0da Compare September 1, 2026 10:29
@jaydeeppatel1111
jaydeeppatel1111 marked this pull request as ready for review September 1, 2026 10:33
@jaydeeppatel1111
jaydeeppatel1111 requested a review from a team as a code owner September 1, 2026 10:33
Copilot AI lite review requested due to automatic review settings September 1, 2026 10:33

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 adjusts CLR/PAL memory reporting and HIP allocation pre-checks for selected large unified-memory APUs so reported total memory, maximum allocation size, and hipMemGetInfo behavior better match the physical UMA layout while protecting a minimum amount of host memory for the OS.

Changes:

  • Add a per-device umaLargeMemory_ setting (enabled for Pal::AsicRevision::StrixHalo) to gate new UMA-specific behavior.
  • In PAL device info, credit the GART aperture more appropriately for UMA (including an OS-reserved floor) and derive maxMemAllocSize_ from the full pool; add detailed PAL memory logging.
  • In HIP ihipMalloc, adjust the max-allocation pre-check logic to avoid double-counting UMA memory.

Reviewed changes

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

File Description
projects/clr/rocclr/device/pal/palsettings.cpp Initializes and enables the new UMA-large-memory gate for validated ASIC revision(s).
projects/clr/rocclr/device/pal/paldevice.cpp Updates global memory sizing/crediting and max allocation derivation for UMA; adds PAL memory diagnostics.
projects/clr/rocclr/device/device.hpp Adds umaLargeMemory_ bit to device settings.
projects/clr/hipamd/src/hip_memory.cpp Updates hipMalloc pre-check to avoid UMA double-counting (per device setting).
Suppressed comments (1)

projects/clr/rocclr/device/pal/paldevice.cpp:362

  • On offline devices, heaps[GpuHeapGartUswc] is not initialized (see NullDevice::create()). Reading it here (and again in the uswcPercentAvailable calculation) is undefined behavior; for offline devices it should be treated as 0.
  const uint64_t gartSize = static_cast<uint64_t>(heaps[Pal::GpuHeapGartUswc].logicalSize);
  const uint64_t hostRAM = amd::Os::getPhysicalMemSize();

  const bool umaLargeMemory = settings().umaLargeMemory_;


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread projects/clr/hipamd/src/hip_memory.cpp Resolved
Comment thread projects/clr/hipamd/src/hip_memory.cpp Resolved
jaydeeppatel1111 and others added 2 commits September 2, 2026 02:25
NullDevice: :create() filled only the Local entry and left the rest of heaps[] indeterminate, so fillDeviceInfo() read undefined values for the invisible and aperture heaps. On offline gfx1151 targets umaLargeMemory_ skips the 64 GiB clamp, letting those values reach globalMemSize_ and maxMemAllocSize_ unbounded. Offline devices now report 512 MiB, as the surrounding comment already claimed.
Co-authored-by: Cursor <cursoragent@cursor.com>
Rework the large-memory APU fix per review:

- Revert hip_memory.cpp entirely. The KMD reports OOM if the sum is
  invalid, so the HIP layer does not need its own guard.
- Drop the per-device umaLargeMemory_ setting. The pre-existing
  apuSystem_ guard already expresses the right condition and covers
  every integrated part without a per-chip whitelist.
- Remove the PAL memory diagnostic.
- Credit the aperture as gart - min(gart/4, 4G) uniformly, replacing
  the 50/75 percent heuristic and the host-RAM floor.
- Raise maxAllocSize_ to 256 GiB rather than skipping the clamp on
  APUs, keeping the bound in place for every device.
- Keep the APU single-allocation ceiling on the whole pool. Without it
  the clamp change is inert and the mempool and pitch paths regress
  below stock on aperture-heavy carve-outs.

Co-authored-by: Cursor <cursoragent@cursor.com>
@jaydeeppatel1111
jaydeeppatel1111 merged commit b4bb73c into develop Sep 4, 2026
106 of 131 checks passed
@jaydeeppatel1111
jaydeeppatel1111 deleted the users/jaypatel/clr-pal-uma-large-memory branch September 4, 2026 14:15
systems-assistant Bot pushed a commit to ROCm/clr that referenced this pull request Sep 4, 2026
 APUs (#10974)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

PAL under-reports memory on large-memory APUs. It discounts the GART
aperture by 25%, and clamps the largest single allocation to a hardcoded
64 GiB regardless of how much memory the part actually has.

## Motivation

**The 64 GiB maximum is a constant, not a measurement.** PAL reports the
same 64.00 GiB on every configuration tested, across carve-outs from 8
GB to 160 GB. The clamp's comment still describes a 4 GB blit-kernel
limit it stopped matching long ago.

**The largest allocation is derived from the largest single heap.** On a
unified memory part that is the wrong quantity. The carve-out and the
aperture are the same DRAM, and a single allocation is not confined to
one heap — measured, see below. Deriving the ceiling from one heap
leaves the rest of the pool unreachable, and does so by a different
amount depending on whether the KMD splits the carve-out.

**The aperture discount has no recoverable rationale.** The aperture is
host DRAM the GPU addresses directly. Discounting it by 25% understates
the pool, and because free memory is derived from the same total, it is
also what makes `hipMemGetInfo` saturate.

## Technical Details

Three changes, all in PAL:
- Credit the aperture as `gart - min(gart/4, 4G)`, replacing the 50/75%
heuristic. This holds back a quarter of the aperture up to a 4 GiB cap,
so the OS is not starved on configurations where the aperture is most of
the pool.
- Derive `maxMemAllocSize_` from the whole pool rather than the largest
single heap.
- Raise `maxAllocSize_` from 64 to 256 GiB, keeping the clamp in place
for every device rather than adding an exception. Without this the
previous change is inert, since the clamp would truncate the
pool-derived value straight back to 64 GiB.

## Issue Tracking

JIRA ID: ROCM-24637

## Test Plan
Dry run the hipMalloc guard on below configs
SH 128 : 96+16+16, with split ON/OFF
SH 128 : 8+96+24
GH : 160+32
GH : 8+184

## Test Result
hipMalloc allows allocation over gart on APUs.

### Direct-map carve-out

The `Gmx_UseFbOffsetAsPaOnApu` regkey decides whether the carve-out
arrives as one segment or is split by BAR round-down. Its default
enables direct-map only when the BAR and carve-out sizes differ, the KMD
document leaves per-milestone enablement unconfirmed, and that
document's own test plan exercises the disabled path — so PAL will meet
both states in the field. Both were tested here.

## Submission Checklist

- [x] Look over the contributing guidelines at
https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.
[rocm-systems] ROCm/rocm-systems#10974 (commit b4bb73c)
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

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.

3 participants