[shwetagkhatri]

JIRA ID : ROCM-31873

GpuAgent::ReserveScratch() pre-binds an optional scratch block that
AcquireQueueMainScratch can later fall back on. When that reservation could not be
satisfied it threw HSA_STATUS_ERROR_OUT_OF_RESOURCES. That exception unwinds the
AqlQueue constructor and surfaces out of whichever API call happens to be the first
to materialize an agent's utility queue - in the reported case hsa_executable_freeze,
which has nothing to do with scratch.

The reservation is an optimization, not a prerequisite. AcquireQueueMainScratch runs
a three stage retry - check the cache, trim unused blocks, trim all blocks - and only
reaches scratch_cache_.use_reserved() as its third attempt. With nothing reserved
that attempt does not help and the function continues to its occupancy-reduction
path. It returns a null base rather than throwing. So a missing reservation does not
break queue creation.

To be clear about what still fails and where: a dispatch that genuinely needs scratch
and cannot obtain it is not silently accepted. AqlQueue::DynamicQueueEventsHandler
turns a null main and alt scratch base into HSA_STATUS_ERROR_OUT_OF_RESOURCES and
delivers it through the queue's error callback. That is the right place and the right
error. What this change removes is the failure of queue CREATION, and the bare 4104
escaping from an unrelated API.

This change replaces both throws with a diagnostic and continues without a reserved
block. The diagnostic is written with fprintf(stderr, ...) behind the existing
enable_queue_fault_message() flag, not debug_print, which is a no-op under NDEBUG and
so would not be visible in a release build.

Precedent. PR #10509, "fix(rocr): validate scratch allocation under constrained
address spaces", merged 2026-09-08, landed commit 626752b whose own subject is
"fix(rocr): survive scratch allocation failure under constrained address spaces".
That change made InitScratchPool tolerate exactly this condition and install an empty
pool instead of asserting. ReserveScratch was not brought along and kept throwing,
which is what this change completes.

How it was hit

rocrtstNeg.Queue_Validation_InvalidDimension and
rocrtstNeg.Queue_Validation_InvalidKernelObject fail under Valgrind on an 8 GPU
gfx950 host. Each agent reserves a 32 GiB scratch VA aperture, 256 GiB in total.
Valgrind's usable address space is much smaller, so InitScratchPool succeeds for the
first two agents and returns HSA_STATUS_ERROR_OUT_OF_RESOURCES for the remaining six,
leaving those agents with an empty scratch pool. Measured on an instrumented build:
the 140 MiB reservation then fails against a pool whose base is null and whose size
is zero, while 287 GiB of device memory is free. Without Valgrind, on the same binary
and host, all eight apertures are obtained and the test passes.

The Valgrind "definitely lost" report named in the ticket is a consequence of this.
The test aborts on the 4104, hsa_shut_down never runs, and the test fixture's own
teardown never runs either. Of the four loss records, two are signals created by
hsa_init that only hsa_shut_down frees, one is a signal allocated by the test itself
in its SetUp, and one is the pair of signals in the queue constructor. With this
change the same command reports 0 bytes in 0 blocks and 0 Memcheck errors, which is
what rules out an independent leak.

A single GPU gfx950 host does not reproduce, because one 32 GiB aperture fits.

Validation

Host: 8 x gfx950, SPX/NPS1, ROCm 10.3.0, valgrind 3.27.1. Both libraries are Release
builds of the same tree, differing only by this patch, confirmed by a binary string
differential before each run.

valgrind --leak-check=full --max-threads=2048 rocrtst64 --gtest_filter=rocrtstNeg.<T>

Test                                  Before            After
Queue_Validation_InvalidKernelObject  FAILED, 520 B /   OK, 0 B / 0 blocks,
                                      5 blocks lost,    0 Memcheck errors
                                      34 errors
Queue_Validation_InvalidDimension     FAILED, 520 B /   OK, 0 B / 0 blocks,
                                      5 blocks lost,    0 Memcheck errors
                                      34 errors

Regression, same-tree A/B, both hosts:

Host         Suite                                      Result
1 x gfx950   rocrtstFunc.* less Max_Reference_Count      94/94 passed, both arms
1 x gfx950   rocrtstNeg.*                                  5/5 passed, both arms
8 x gfx950   rocrtstFunc.* less Max_Reference_Count      94/94 passed, both arms

No test changes state between the arms on either host. Max_Reference_Count is
excluded only because it runs for well over 30 minutes.

Not covered: the Valgrind A/B above is gfx950 only. A separate functional A/B at the
current merge base was run on 8 x gfx942; it does not re-measure the Valgrind figures.

Rebase note: the validation above was run on this patch applied to develop at
f8b5dd4. The branch has been rebased twice since, onto 1e8c6de and then onto
f030609. Neither the Valgrind A/B nor the gfx950 regression table above has been
re-run against either base. A functional
A/B of rocrtstFunc.* minus the Interprocess and IPC groups was run at f030609 on
8 x gfx942 in the normal configuration and was 97 of 97 on both arms. The patch body was
byte-identical across the first rebase. It is NOT identical after the second: in
response to review the diagnostic moved from debug_print to fprintf behind the
queue-fault flag, the map-failure branch gained an explicit return, and the added
comment was corrected. A follow-up commit then guarded the empty-pool high-split read
that the no-reservation case exposes.

Across the first range, f8b5dd4..1e8c6de, exactly one commit touches the
patched file: a9c3b00, "fix(rocr): Select I$ and GL2 invalidation for gfx125x code
caches (#13924)". Its new branch in InvalidateCodeCaches is gated on GFX major 12 /
minor 5, and its else branch replaced a literal 1 with
PM4_ACQUIRE_MEM_GCR_CNTL_GLI_INV_ALL, which is defined as 1U. Neither is reachable as a
behaviour change on gfx950.

Across the second range, 1e8c6de..f030609, no commit touches the patched file.
Other rocr-runtime commits did land in that range and are linked into the same library,
so this is a narrower statement than a re-run would be.

🤖 Generated with Claude Code

[therock-pr-bot]

✅ 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/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_gpu_agent.cpp, projects/rocr-runtime/runtime/hsa-runtime/core/util/small_heap.h; 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?

[cfreeamd]

🤖 AI-assisted pre-review (for @cfreeamd)

The GpuAgent::ReserveScratch() change looks functionally correct. Verified against develop source that AcquireQueueMainScratch performs full on-demand scratch allocation (cache → fresh scratch_pool_.alloc/MakeMemoryResident → trim), using the reserved block only as a late retry tier — so dispatch is unaffected when nothing is reserved. Cleanup is sound: on map failure scratch_cache_.reserve() is never called (no accounting corruption) and scratch_pool_.free() correctly undoes the alloc. The sole caller in amd_aql_queue.cpp is wrapped by a QueueGuard scope guard so removing the throw does not break queue teardown.

[Required] Title format: rocr: do not fail queue creation when the scratch reservation fails does not follow the required type(component): convention. ROCm squash-merges, so this becomes the permanent commit title. Please retitle to: fix(rocr): do not fail queue creation when the scratch reservation fails

[Question] Missing PR description: This flips queue-creation semantics from fail-hard to succeed-silently. A brief description explaining the scenario where the reservation legitimately fails (e.g. memory pressure / fragmentation) and any perf measurement would help reviewers verify the trade-off.

[Copilot]

🟡 Changes recommended

Failed scratch VA allocation can leave a queue with an empty SmallHeap, and a later unguarded high_split() call dereferences the empty set, causing undefined behavior instead of reporting out-of-resources.

1 open issue

• Guard high_split when scratch allocation fails · New

What changed in this PR

Makes ROCr’s optional scratch reservation nonfatal so reservation failures do not abort queue creation or escape through unrelated APIs, extending the empty-pool handling introduced in #10509.

Changes:

• Replaces allocation and mapping failure exceptions with optional diagnostics and early returns.
• Preserves cleanup when mapping a reserved block fails.
• Documents that scratch reservation is an optimization, not a queue-creation requirement.

File	Description
projects/​rocr-runtime/​runtime/​hsa-runtime/​core/​runtime/​amd_gpu_agent.cpp	Allows ReserveScratch() to fail without throwing, with flag-controlled diagnostics.

_{Review effort: Balanced}

[Copilot]

🟢 Approval recommended

No active review issues block the changes that make optional scratch reservation failures nonfatal.

No open issues 🎉

1 resolved since last review

• Guard high_split when scratch allocation fails

_{Review effort: Balanced}