DO NOT MERGE - For Discussion - #11
Closed
AWoloszyn wants to merge 90 commits into
Closed
Conversation
This allows pytorch to complete without crashing.
* Rebase on top of benvanik/users/benvanik/amdgpu-wip This currently builds and runs all of the tests with hsa. It does not actually turn on the amdgpu driver yet, but everything builds with the new API. The biggest missing feature here is that we don't expose a reasonable pool API. For every buffer that we create we create a new pool based on the buffer params that were passed in. This passes all tests, but we need an update to the public API to make this nicer. * Some debug logging and enabling of AMDGPU * Switch to AMDGPU and delete reference HSA driver. * Add profiler knob and add CB barriers in the low-level API. * Add hrx_stream_wait() convenience function. * Add trace zones to the public API --------- Co-authored-by: Andrew Woloszyn <andrew.woloszyn@gmail.com>
* Install hrx-info * Install CTS tests to share/hrx-cts * Have CTS locate testdata relative to itself. * Make CUDA binding optional and OFF by default. * Set ORIGIN relative RPATH for installed cts.
Reapplies the HRX-local patch stack on top of the new pristine import (now pinned to users/awoloszyn/hrx_on_amdgpu, IREE a56a07822441). The host_queue_command_buffer_test guard had to be hand-applied because the test's SRCS list moved from host_queue_command_buffer_internal.h to host_queue_command_buffer_packet.h on the new base.
IREE's AMDGPU driver pulls in <aqlprofile-sdk/aql_profile_v2.h> from the rocprofiler-sdk distribution, which is not part of the base ROCm runtime package. On ROCm 7.x system installs this header is missing under /opt/rocm/include/aqlprofile-sdk/, so IREE fails to compile when the HAL_DRIVER_AMDGPU is enabled. Vendor the two headers we need under third_party/aqlprofile-sdk/include (taken verbatim from IREE's previous third_party/hsa-runtime-headers bundle) and append that include path to the hsa_runtime_headers interface target our HRX-side IREE patch creates. The path is wrapped in $<BUILD_INTERFACE:...> so it is only used at build time and does not leak into the installed CMake export set.
…h2 in CTS
A handful of small but real correctness fixes that fall out of running
the streaming integration end to end:
* Store the full iree_hal_buffer_mapping_t on hrx_buffer_s instead of
only its data pointer, and use it in hrx_buffer_unmap and the
ref_count==1 cleanup path. Reconstructing a mapping struct on the fly
with just .contents.{data,data_length} loses the IREE-side cookie and
causes iree_hal_buffer_unmap_range to leak/mishandle the underlying
scoped mapping.
* Have hrx_device_retain also retain the underlying
iree_hal_device_group_t. The release path was already releasing both
the device and the group, so retain was unbalanced by one on every
call.
* In cts/CMakeLists.txt, FetchContent Catch2 v3.8.1 instead of relying
on a system-installed Catch2 3.x. Most ROCm dev images don't ship
Catch2 packages, so a fresh checkout could not configure the CTS at
all. find_package(Catch2 3 REQUIRED) is left commented as a pointer
to the alternative if a system package is preferred later.
Bring over the libhrx-side support for the streaming integration: - `hrx_event_t` plus timing / query / wait APIs. - `hrx_graph_t` + `hrx_graph_exec_t` with kernel, memcpy, memset, host-call, and nested-graph node kinds, topological analysis, scheduling, and execution. Static helpers that compose IREE primitives return `iree_status_t` internally; public API entry points convert at the boundary via `hrx_status_from_iree`. - `hrx_mem_pool_t` backing the streaming memory pool surface. - `hrx_buffer_table_t`, a device/host-pointer lookup table used for pinned-host registration and device-pointer -> buffer resolution. - Host-memory registration (`hrx_host_memory_register/unregister`) and buffer-by-device-pointer lookup (`hrx_buffer_lookup`). - Async device<->host transfer helpers `hrx_stream_copy_h2d` / `hrx_stream_copy_d2h` with chunking. - Device memory info query and peer-access probing. - `HRX_RETURN_IF_IREE_ERROR` / `HRX_RETURN_AND_END_ZONE_IF_IREE_ERROR` macros so libhrx functions returning `hrx_status_t` can cleanly wrap IREE calls without confusing the two status representations. Also wires per-device `iree_arena_block_pool_t` and `hrx_buffer_table_t` initialization into `hrx_cpu_initialize` / `hrx_gpu_initialize` (with matching teardown), and stores the full `iree_hal_buffer_mapping_t` on `hrx_buffer_s` so map/unmap correctly balances the transient-buffer retain/release.
…ring properties / module globals
Physically relocate the streaming layer from src/streaming/ to
src/binding/common/ alongside the HIP and CUDA bindings, and add the
new mem_pool glue (src/binding/common/mem_pool.c) built on top of
hrx_mem_pool_t. The top-level CMakeLists.txt picks up the new
subdirectory, and the old src/streaming/ tree (including the
per-streaming buffer_table and its benchmark/test) is removed now
that libhrx owns the buffer table.
As part of this move, two APIs that previously tried to delegate to
IREE HAL entry points that no longer exist are resolved entirely
inside the streaming layer:
- iree_hal_streaming_device_get_string_property now reads from values
cached on iree_hal_streaming_device_t at init time
(device->info.name, device->info.path, device->gcn_arch_name) for
the ("hal.device", {"name","path","architecture","gcn_arch_name"})
keys, with IREE_STATUS_NOT_FOUND / IREE_STATUS_OUT_OF_RANGE
contracts. The signature is tightened to take const char* for
category/key. This removes the dependency on the (deleted)
iree_hal_device_query_string HAL entry point.
- iree_hal_streaming_module_global now resolves exclusively out of
module->symbols[] (populated by iree_hal_streaming_module_extract_
metadata via iree_hal_executable_export_info/_export_parameters).
The stale iree_hal_executable_lookup_global_by_name fallback and
its ad-hoc transient-buffer registration path are deleted.
The IREE HAL is intentionally left without by-name global lookup or
string-property query; those are streaming-layer concerns.
Point the libamdhip64 and libcuda shared libraries at the new hrx_binding_common target (moved from the old hrx_streaming subdirectory) and pick up the new event/graph/mem_pool/host-memory entry points exposed by the streaming layer. Adds binding-private helpers in src/binding/hip/binding_internal.h that were previously tangled into api.c. Explicitly drops the CUDA/HIP fat-binary (CCOB) / ZSTD code paths: raw ELF binaries are passed through to AMDGPU at the streaming layer instead of being repackaged in the bindings, matching the direction of the new streaming integration.
Register the new cts/tests/mem_pool/ test target and update the CTS loader to pick up the streaming-integration APIs (events, graphs, memory pools, host memory registration). Gives us coverage of hrx_mem_pool_t end-to-end from the CTS harness.
First pass at the support libamdhip64 needs to actually load
real-world HIP fat binaries (rocBLAS/hipBLAS/Tensile, the PyTorch
distribution kernels, etc.). Previous bring-up only handled raw ELF
objects; clang's __hip_fatbin payloads ship as Clang Offload Bundles
(CCOB), often with the per-target ELFs zstd-compressed.
* New src/binding/common/fat_binary.{c,h} implementing a CCOB parser
that walks the bundle header, picks the entry whose target_id
matches our gfx*-generic device, and returns a borrowed pointer (or
decompressed copy) of the embedded ELF.
* CMake: lift the IREE_HAL_DRIVER_ENABLE_ZSTD knob out of the top-level
CMakeLists.txt and turn it into a proper HRX_ENABLE_ZSTD option
(still ON by default). The hrx_binding_common target now does its
own find_package(zstd) and links libzstd into the streaming layer
conditional on the option, so building HRX without zstd cleanly
disables CCOB decompression rather than failing the IREE config.
* Streaming module loader (src/binding/common/module.c) plumbs the
fat-binary path: when given a Clang Offload Bundle blob it walks
entries, decompresses the matching one if needed, and feeds the
resulting ELF straight into the existing executable-load path.
* hipModuleLoadData / hipModuleLoadDataEx in src/binding/hip/api.c
forward to the new path.
* Minor surface plumbing in hrx_bridge.h / internal.h to expose the
device's gcn_arch_name and to thread the bundle entry through to
the streaming layer.
* libhrx/runtime.c: a few cleanups landed here while wiring up the
binding side, including better error reporting on initialize/shutdown
and refactoring the profile_data_families plumbing the wip commit
introduced.
…ffset
Three closely related streaming-layer fixes pulled together so PyTorch
inference (gpt2_infer / gpt2_long) produces non-NaN logits on the
AMDGPU HAL backend.
* iree_hal_streaming_launch_kernel now passes a real
iree_hal_memory_barrier_t to iree_hal_command_buffer_execution_barrier
with both source_scope and target_scope set to
DISPATCH_READ|DISPATCH_WRITE|TRANSFER_READ|TRANSFER_WRITE. Previously
the call passed count=0, ptr=NULL, which after IREE
48af1651a1 ("Preserve command-buffer barrier scopes") leaves the
AQL release/acquire fences at SCOPE_NONE and skips the AQL barrier
bit entirely on the trailing payload packet. That makes
back-to-back dispatches racy with respect to L1/L2 visibility and
manifests as silent NaNs through transformer-style kernels. With
AGENT-scoped fences explicitly requested through the proper memory
barrier, the AMDGPU backend emits the correct release+acquire pair
between dispatches.
* iree_hal_streaming_module_extract_metadata: drop the dual-offset
hack for BINDING parameters. Use parameter->offset uniformly for
both src->dst dispatch resolve ops and the kernarg buffer-extent
computation now that the AMDGPU HSACO metadata reports a real byte
offset there, instead of routing BINDINGs through |kernarg_offset|.
* Strip the IREE_STREAMING_DEBUG_KERNEL_LAUNCH debug logging block
from iree_hal_streaming_launch_kernel and a stale comment in
hipLaunchKernel. They were only useful while bisecting the NaN
regression and add noise + a perf hit on every dispatch.
Two follow-on fixes to the streaming-rebase commits so the CTS actually compiles: * Add the missing decltype member declarations to HrxLoader for the event/stream_wait_event APIs and the mem_pool APIs that d4b5952 ("cts: add mem_pool test suite and update loader for new APIs") wired into hrx_loader.cpp without ever touching the header. * Rename HRXTestFixture references in the mem_pool test suite to HrxTestFixture, matching the actual class declared in hrx_test_fixture.hpp. After this all 17 CTS binaries build and pass against the default cpu:0 device. Most also pass against gpu:0; the remaining two failures (hrx_cts_lifecycle's CPU lifecycle cases when launched in GPU mode and hrx_cts_executable's HIP compile, which needs ROCm's clang rather than our local LLVM build) are independent of this fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.