Summary:
This provides users with a way to check all symbols present in a given
program. This is useful for many cases where a power user wants to do
something dynamic with the objects present.
We need to pass the right linker option and not try to containerize it.
Fixes two OpenMP offload JIT tests, the other has some front end assert
I will look at.
Signed-off-by: Nick Sarnie <nick.sarnie@intel.com>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`packAndTransfer()` called `submitData()`, which captured the address of
the data of
the `SmallVector` member of `PrivateArgumentManagerTy`, as the host
source of an
async transfer. The manager was later moved into an `AsyncInfo`
post-processing
lambda, changing the underlying address of the data:
`addPostProcessingFunction()`
copies the closure rather than moving it, but even if it "moved", the
address of the data owned by `SmallVector`'s inline capacity would have
changed.
The AMDGPU plugin read from the stale host source address when doing the
actual transfer.
Fix: allocate the packed buffer via
`getOrCreateSourceBufferForSubmitData()`, so
that `AsyncInfo` owns it. The non-packed
corresponding-pointer-initialization
path in this same class already built its source buffer that way.
Fails with an error about not being able to find `lld`. AMD team is
investigating, but disable the test in the meantime.
Context: https://github.com/llvm/llvm-project/pull/212860
Signed-off-by: Nick Sarnie <nick.sarnie@intel.com>
Description:
This change enables the offload test build on Windows by adding
compatibility guards around libomptarget test suites.
Why:
libomptarget is not available on Windows. Without these guards, CMake
fails when referencing the missing omptarget target. With these guards,
Windows builds complete successfully and run available tests (using
check-offload-unit), while Linux/macOS builds remain unaffected.
* Add a few tests for when a mapper does something like `map(s.p[0:10])`
where `p` is a pointer.
* Add a few tests that require propagation of bits like `present/always`
into a mapper.
* Fix a few tests that were expecting `p` to be mapped when the mapper
only said `map(s.p[0:10])`.
The output of a few tests is different from what we expect. They have
been annotated with FIXMEs, and the expected output for when the
follow-up changes in this stack to propagate the map-type-modifier bits
and using attach-style mapping for mappers get merged.
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
I tried to do `check-offload` instead but that
[caused](https://github.com/llvm/llvm-project/pull/212500) a bloodbath,
so just enable `check-offload-unit` as part of `check-all`.
Signed-off-by: Nick Sarnie <nick.sarnie@intel.com>
It's unclear why these were excluded from `check-all`, but we made all
tests pass on Level Zero and our local testing shows they pass on AMD
and NVIDIA too, so enable it by default.
Context: https://github.com/llvm/llvm-project/pull/211633
Signed-off-by: Nick Sarnie <nick.sarnie@intel.com>
PR #205224 changed the path for kernels with no arguments to not use
appendLaunchKernelWithArgs, while at the same time it removed
zeKernelSetGroupSize which resulted in incorrect sizing of the kernel.
This PR removes the alternate path and makes sure all launches go
through appendLaunchKernelWithArgs.
Alternatively we can restore zeKernelSetGroupSize on the alternate path.
At the previous community meeting, we agreed to introduce contexts into
the liboffload API. I also mentioned that, as part of this work, we
could split the allocation function into three separate functions, but
we did not discuss this idea in detail.
This patch only splits the allocation functions, allowing us to discuss
and agree on the proposed interface before introducing contexts into the
`olMemAlloc` API.
Once contexts are added, the device parameter will be removed from host
allocations, and the final interface would look like this:
```c
ol_result_t olMemAllocDevice(ol_context_handle_t, ol_device_handle_t, size_t, void **);
ol_result_t olMemAllocHost(ol_context_handle_t, size_t, void **);
ol_result_t olMemAllocManaged(ol_context_handle_t, ol_device_handle_t, size_t, void **);
```
Currently, for host allocations, the device parameter is required mainly
to determine which plugin should perform the allocation. Once contexts
are introduced, this information will be provided by the context, making
the device parameter redundant.
Requiring both a context and a device for host allocations could also be
misleading, since it is unclear what the device parameter actually
represents in this API.
Assisted by: Claude.
This patch does a small cleanup of level zero plugin. Mainly I changed
names of variables to use `CamelCase` whenever they relate to our
internal structures - not objects coming directly from level zero API
etc., so I left some variables and arguments like `zeDevice`, but only
if they are directly related to level zero APIs or structures.
Added a few `auto *` declarations, but again - only when they refer to
our internal objects and not to structures from level zero which might
be pointers underneath, but we use them as transparent objects.
Also did small cleanup of unused headers, making sure that we include
directly what we need rather than use transitive dependencies.
@adurang Please let me know if this patch is okay with you, if you think
this will cause more harm then it's worth I'll close it.
---------
Co-authored-by: Łukasz Plewa <lukasz.plewa@intel.com>
Add the initial Context handle API to liboffload:
- olCreateContext / olDestroyContext
- olGetContextInfo / olGetContextInfoSize
- OL_ERRC_INVALID_CONTEXT error code
- OL_CONTEXT_INFO_{NUM_DEVICES,DEVICES,PLATFORM} queries
This is the first patch in a series. I decided to split the original
change into multiple smaller PRs to make the review process faster and
more manageable.
This patch introduces context management functions, but the context
handle is not used anywhere yet. The following patches will extend the
API to use contexts in:
``` c
ol_result_t olMemAlloc(ol_context_handle_t, ol_device_handle_t, ol_alloc_type_t, size_t, void**);
ol_result_t olMemFree(ol_context_handle_t, void*);
ol_result_t olGetMemInfo(ol_context_handle_t, const void*, ol_mem_info_t, size_t, void*);
ol_result_t olGetMemInfoSize(ol_context_handle_t, const void*, ol_mem_info_t, size_t*);
ol_result_t olCreateQueue(ol_context_handle_t, ol_device_handle_t, ol_queue_handle_t*);
ol_result_t olCreateProgram(ol_context_handle_t, ol_device_handle_t, const void*, size_t, ol_program_handle_t*);
```
You can also take a look at #201398, which contains the first version of
the complete change. I'm cleaning it up as I go, so the patches in this
series are not a direct 1:1 split of that PR.
`__tgt_register_rpc_callback` unconditionally called
`Plugin::getRPCServer()` which asserts if the RPC server pointer is
null. When a plugin is initialized but its RPC server has not been
created (e.g., `NumDevices == 0` in `GenericPluginTy::init()`), this
triggers a crash.
Rather than introducing a nullable accessor, guard the call by also
checking that the plugin has at least one device. A zero-device plugin
has no RPC server to register callbacks with, so the extra check is both
correct and sufficient.
Suggested by @jhuber6 in the review of the previous attempt (#210215).
Summary:
This is a holdover from how we used to handle this stuff. We should just
dirctly check the triple for setting the cross-compiling emulator and we
can directly introspect into the build tree for the lit config like we
already do for flang-rt.
The message was the opposite of the assert condition.
In addition, when PatternSize == Size the caller wants to fill the
region with exactly one copy of the pattern, which is a valid operation.
The issue was found by AI.
Also check `Size % PatternSize == 0` per L0 spec.
Even though L0, CUDA and HSA do not directly support filling memory
regions with non-power-of-two-sized patterns, plugins explicitly allow
for such fills by falling back to a slow path whenever a direct API call
is not possible.
The Level Zero plugin is the odd one out, forwarding directly to
`zeCommandListAppendMemoryFill`. This fails for non-power-of-two
patterns, causing unit tests for `olMemFill` to fail when run on L0.
This PR adds fallbacks in L0 plugin's `memoryFill` implementation to
support arbitrary pattern sizes:
- if all pattern bytes are the same, substitute `1` for `patternSize`
and proceed with the native fill
- if memory is host-accessible (host or shared alloc), then simply
`std::copy_n` the pattern
- if memory is device-only, upload a seed to the device, and
`memoryCopy` O(log n) times, doubling the pattern each time, until the
entire region is filled. The seed is the input pattern, but extended on
the host to 1-2KiB to potentially eliminate several small `memoryCopy`
calls that would otherwise be almost pure overhead.
Conceptually, this approach has better worst-case overhead than the one
found in the CUDA plugin (decompose the pattern to then run multiple
strided memsets) and the AMDGPU one (call `pushMemoryCopyH2DAsync` with
the pattern, setting `NumTimes`). Aligning the fallback fill approaches
is outside the scope of this PR, however - the main intent is to fix the
unit test failures.
Tracking issue: #201242
See the [migration guide] for more information.
[migration guide]:
https://llvm.org/docs/SphinxQuickstartTemplate.html#markdown-migration-guidelines
This is a stacked PR based on #210208, which is the standalone rename
commit for this miscellaneous Markdown migration batch.
This was prepared with rst2myst plus LLM-assisted cleanup. LLDB man
pages are intentionally left as reStructuredText because LLDB man-page
building should not take on Markdown parser dependencies.
Unfortunately, many of these miscellaneous docs do not have working
standalone doc build targets, so I was not able to render HTML for all
of them. I think the scope is small enough that this is reviewable by
hand.
When a Level Zero driver is not present in the environment the current
behavior makes olInit fail if the level_zero plugin is requested. This
changes the behavior to match other plugins where it doesn't fail but
return 0 supported devices.
https://github.com/llvm/llvm-project/pull/207208 caused a CI failure on
openmp-offload-amdgpu-clang-flang in the
olCreateProgramTest.TruncatedBinary test.
This PR removes this test case, as it's unclear whether guarding against
such cases is worth it.
liboffload plugins would not always exhaustively map the underlying
API's error values to ol_errc_t, resulting in unintended behavior - e.g.
olCreateProgram, given an invalid binary, would return OL_ERRC_UNKNOWN
instead of OL_ERRC_INVALID_BINARY.
This patch largely eliminates the most glaring cases of this by
assigning an ol_errc_t value to every error code of the underlying API
in every plugin. The proper granularity for this mapping is per-API
call, but that is a large rework that belongs in a separate patch.
Assisted-by: Claude
This fixes Fortran `declare target enter(automap:...)` handling for
allocatable
descriptors.
- Replace the previous `has_device_addr` target-region rewrite with
explicit
target data operations around allocation/deallocation.
- On allocation, emit:
- descriptor `map(always,to)` to keep the Fortran descriptor present
- storage `map(storage)` for the allocated array data
- `map(attach)` to update the device descriptor base address to the
device data
- On deallocation, delete both the mapped storage and descriptor.
- Add a weak visible offload-entry alias for local/hidden `declare
target to/enter`
globals so libomptarget can associate the host descriptor with the real
device
global without changing the original symbol linkage.
This fixes `offload/test/offloading/fortran/declare-target-automap.f90`
reported broken in #161265.
Co-authored-by: Codex <codex@openai.com>
This PR attempts to fix the error:
```
llvm-lit: /home/botworker/builds/openmp-offload-amdgpu-runtime-2/llvm.src/llvm/utils/lit/lit/discovery.py:273: warning: input '/home/botworker/builds/openmp-offload-amdgpu-runtime-2/build/llvm.build/runtimes/runtimes-amdgpu-amd-amdhsa-bins/libc/test' contained no tests
error: did not discover any tests for provided path(s)
```
According to lib/test/CMakeList.txt, we need to set
`CMAKE_CROSSCOMPILING_EMULATOR` to avoid early return in the cmake. It
only changes the build script instead of the parent cmake cache file.
Not sure if this is the optimal solution, but it works for us.
Summary:
Quite important that this works before the fork. The problem is that
CMake takes the string literally while rewriting it to amdgpu/
internally so the amdgcn/ directories do not appear.
After the amdgcn-amd-amdhsa -> amdgpu-amd-amdhsa rename, look up the
device runtime libraries in the legacy directory when the runtime was
built under the old triple name.
Claude assisted with this patch.
In __kmpc_target_init, thread 0 initializes the team state in LDS
(HasThreadState = 0, ThreadStates = nullptr) and then releases the other
threads with synchronize::threadsAligned(atomic::relaxed). Because the
relaxed ordering emits no memory fence, the initializing LDS stores are
not guaranteed to be visible to the worker threads before they proceed.
A worker thread can then read the uninitialized team state and produce
wrong results or crash. This is the flow that causes the race as I
understand it.
```
__kmpc_target_init (Kernel.cpp:107)
-> initializeRuntime (Kernel.cpp:38)
-> state::init (State.cpp:286)
-> TeamState.init(IsSPMD) (State.cpp:295)
-> HasThreadState = false (State.cpp:248) // <-- the value later read stale
ThreadStates = nullptr (State.cpp:296)
-> synchronize::threadsAligned(atomic::relaxed) (Kernel.cpp:115) // <-- insufficient barrier:
// relaxed emits no fence, so the state stores above are not published to the other threads
... worker threads continue past the barrier ...
__kmpc_parallel_60 (Parallelism.cpp:143)
if (OMP_UNLIKELY(!if_expr || state::HasThreadState || ...)) (Parallelism.cpp:158)
^ reads HasThreadState from LDS; a stale/unpublished value (!= 0)
diverts the worker onto the wrong (serialized) path
```
The fix changes the barrier at Kernel.cpp:115 from atomic::relaxed to
atomic::acq_rel so that the memory fences are emitted.
Reproducing the problem requires an `if` clause with a non-constant
condition on the target region. The worker's read of HasThreadState
originates from the serialized-region check in __kmpc_parallel_60. When
the `if` condition is constant (or absent), the optimizer can prove that
branch is dead and eliminates the HasThreadState load entirely, so the
race has nothing to observe. A non-constant `if` keeps if_expr, and
hence the LDS read alive, exposing the problem.
I have added a Fortran offload regression test that launches a target
region many times to make the timing-dependent race likely to surface.
Fixes https://github.com/llvm/llvm-project/issues/208698.
Assisted-by: Cursor
This PR attempts to address the remaining flang offload test failure
after https://github.com/llvm/llvm-project/pull/208617. Bot:
https://lab.llvm.org/buildbot/#/builders/67/builds/8412
The problem with is test is that `foo__l8` kernel was not linked into
device image without explicitly use the amdgpu-amd-amdhsa triple in the
compilation. It only happened to this specific test.
Local test results after fix:
```
Testing Time: 146.44s
Total Discovered Tests: 3478
Skipped : 77 (2.21%)
Unsupported : 341 (9.80%)
Passed : 3055 (87.84%)
Expectedly Failed: 5 (0.14%)
```
Whilst there is no data in theory to map with a zero sized array, the
Fortran OpenMP specification considers the case of a zero sized array
being mapped to device as the data being present on device. This
includes both the data and the descriptor, and it should fundamentlaly
work as a zero-sized array would on device with the respective Fortran
functions for presence and size checking etc.
We can't actually map a nullptr to device and expect it to be present in
current OpenMP (for good reason), however, thankfully, a zero sized
array isn't actually a nullptr, it is a descriptor contianing an
allocated 1-byte of data. So, we can map this to device, alongside the
descriptor and then the zero sized array is correctlly on device for all
intents and purposes. This case is notably different from a
non-allocated or non-zero sized array so we can do this without shooting
ourselves in the foot.
Offload tests failed after introducing the new triple. This PR attempts
to address that. It includes the changes in
https://github.com/llvm/llvm-project/pull/208551. In addition, it also
made following changes:
- Updated flang builder that our buildbot uses;
- Updated the CMakeLists for offload and AMDGPU device runtime (a little
duplications to avoid breaking other buildbots);
- Added per target linker path and mapping for lit substitution/features
in lit config;
- Updated hard-coded check lines in the tests.
This PR reduces the failed offload tests from 246 to 2 on my test runs
locally. Test results and the two failed tests:
```
Failed Tests (2):
libomptarget :: amdgpu-amd-amdhsa :: offloading/fortran/declare-target-common-block-main.f90
libomptarget :: amdgpu-amd-amdhsa :: sanitizer/use_after_free_2.c
Total Discovered Tests: 3478
Skipped : 77 (2.21%)
Unsupported : 341 (9.80%)
Passed : 3053 (87.78%)
Expectedly Failed: 5 (0.14%)
Failed : 2 (0.06%)
```
I was not able to identify the commits for the two failed tests yet
(didn't look like flakiness), will try again later.
Co-authored-by: Jan Patrick Lehr <JanPatrick.Lehr@amd.com>
Adds olMemPrefetch and the OL_MEM_MIGRATION_FLAG_* enum, wired through a
new dataPrefetchImpl on the plugin interface with Level Zero and CUDA
implementations. Backends without prefetch support inherit the default
no-op.
Prefetch is a hint that migrates a USM allocation between the host and a
device so subsequent accesses on the target side are faster.
Assisted-by: Claude
The PGO tests expect the produced filename to match the triple.
The tool pulls the triple name from the binary's triple name, and the
RUN lines expect it to match the triple the build was configured with.
