[Libomptarget][NFC] Make Libomptarget use the LLVM naming convention

Libomptarget grew out of a project that was originally not in LLVM. As
we develop libomptarget this has led to an increasingly la

[Libomptarget][NFC] Make Libomptarget use the LLVM naming convention

Libomptarget grew out of a project that was originally not in LLVM. As
we develop libomptarget this has led to an increasingly large clash
between the naming conventions used. This patch fixes most of the
variable names that did not confrom to the LLVM standard, that is
`VariableName` for variables and `functionName` for functions.

This patch was primarily done using my editor's linting messages, if
there are any issues I missed arising from the automation let me know.

Reviewed By: saiislam

Differential Revision: https://reviews.llvm.org/D128997

show more ...

[Libomptarget] Fix LIBOMPTARGET_INFO test

Summary:
A patch added a new line to one of the info outputs without updating
this test. This patch adds the new text to the existing test.

[libomptarget] Implement pointer lookup as 5.1 spec.

As described in 5.1 spec
2.21.7.2 Pointer Initialization for Device Data Environments

Reviewed By: RaviNarayanaswamy

Differential Revision: htt

[libomptarget] Implement pointer lookup as 5.1 spec.

As described in 5.1 spec
2.21.7.2 Pointer Initialization for Device Data Environments

Reviewed By: RaviNarayanaswamy

Differential Revision: https://reviews.llvm.org/D123093

show more ...

[OpenMP][FIX] Avoid races in the handling of to be deleted mapping entries

If we decided to delete a mapping entry we did not act on it right away
but first issued and waited for memory copies. In t

[OpenMP][FIX] Avoid races in the handling of to be deleted mapping entries

If we decided to delete a mapping entry we did not act on it right away
but first issued and waited for memory copies. In the meantime some
other thread might reuse the entry. While there was some logic to avoid
colliding on the actual "deletion" part, there were two races happening:

1) The data transfer back of the thread deleting the entry and
the data transfer back of the thread taking over the entry raced.
2) The update to the shadow map happened regardless if the entry was
actually reused by another thread which left the shadow map in a
inconsistent state.

To fix both issues we will now update the shadow map and delete the
entry only if we are sure the thread is responsible for deletion, hence
no other thread took over the entry and reused it. We also wait for a
potential former data transfer from the device to finish before we issue
another one that would race with it.

Fixes https://github.com/llvm/llvm-project/issues/54216

Differential Revision: https://reviews.llvm.org/D121058

show more ...

[OpenMP][FIX] Ensure exclusive access to the HDTT map

This patch solves two problems with the `HostDataToTargetMap` (HDTT
map) which caused races and crashes before:

1) Any access to the HDTT map n

[OpenMP][FIX] Ensure exclusive access to the HDTT map

This patch solves two problems with the `HostDataToTargetMap` (HDTT
map) which caused races and crashes before:

1) Any access to the HDTT map needs to be exclusive access. This was not
the case for the "dump table" traversals that could collide with
updates by other threads. The new `Accessor` and `ProtectedObject`
wrappers will ensure we have a hard time introducing similar races in
the future. Note that we could allow multiple concurrent
read-accesses but that feature can be added to the `Accessor` API
later.
2) The elements of the HDTT map were `HostDataToTargetTy` objects which
meant that they could be copied/moved/deleted as the map was changed.
However, we sometimes kept pointers to these elements around after we
gave up the map lock which caused potential races again. The new
indirection through `HostDataToTargetMapKeyTy` will allows us to
modify the map while keeping the (interesting part of the) entries
valid. To offset potential cost we duplicate the ordering key of the
entry which avoids an additional indirect lookup.

We should replace more objects with "protected objects" as we go.

Differential Revision: https://reviews.llvm.org/D121057

show more ...

[OpenMP] Allow to explicitly deinitialize device resources

There are two problems this patch tries to address:
1) We currently free resources in a random order wrt. plugin and
libomptarget destru

[OpenMP] Allow to explicitly deinitialize device resources

There are two problems this patch tries to address:
1) We currently free resources in a random order wrt. plugin and
libomptarget destruction. This patch should ensure the CUDA plugin
is less fragile if something during the deinitialization goes wrong.
2) We need to support (hard) pause runtime calls eventually. This patch
allows us to free all associated resources, though we cannot
reinitialize the device yet.

Follow up patch will associate one event pool per device/context.

Differential Revision: https://reviews.llvm.org/D120089

show more ...

[OpenMP][NFCI] Use RAII lock guards in libomptarget where possible

Differential Revision: https://reviews.llvm.org/D121060

Revert "[OpenMP][NFCI] Use RAII lock guards in libomptarget where possible"

This reverts commit ff50e81b500800708db927cbccca2ab52ec11884 as it broke
the buildbots, see https://reviews.llvm.org/D1210

Revert "[OpenMP][NFCI] Use RAII lock guards in libomptarget where possible"

This reverts commit ff50e81b500800708db927cbccca2ab52ec11884 as it broke
the buildbots, see https://reviews.llvm.org/D121060#3362737.

show more ...

[OpenMP][NFCI] Use RAII lock guards in libomptarget where possible

Differential Revision: https://reviews.llvm.org/D121060

[OpenMP] Avoid costly shadow map traversals whenever possible

In the OpenMC app we saw `omp target update` spending an awful lot of
time in the shadow map traversal without ever doing any update the

[OpenMP] Avoid costly shadow map traversals whenever possible

In the OpenMC app we saw `omp target update` spending an awful lot of
time in the shadow map traversal without ever doing any update there.
There are two cases that allow us to avoid the traversal completely.
The simplest thing is that small updates cannot (reasonably) contain
an attached pointer part. The other case requires to track in the
mapping table if an entry might contain an attached pointer as part.
Given that we have a single location shadow map entries are created,
the latter is actually fairly easy as well.

Differential Revision: https://reviews.llvm.org/D113124

show more ...

[OpenMP] Introduce an environment variable to disable atomic map clauses

Atomic handling of map clauses was introduced to comply with the OpenMP
standard (see D104418). However, many apps won't need

[OpenMP] Introduce an environment variable to disable atomic map clauses

Atomic handling of map clauses was introduced to comply with the OpenMP
standard (see D104418). However, many apps won't need this feature which
can be costly in certain situations. To allow for applications to
opt-out we now introduce the `LIBOMPTARGET_MAP_FORCE_ATOMIC` environment
flag that voids the atomicity guarantee of the standard for map clauses
again, shifting the burden to the user.

This patch also de-duplicates the code that introduces the events used
to enforce atomicity as a cleanup.

Differential Revision: https://reviews.llvm.org/D117627

show more ...

[OpenMP][Offloading] Fixed a crash caused by dereferencing nullptr

In function `DeviceTy::getTargetPointer`, `Entry` could be `nullptr` because of
zero length array section. We need to check if it i

[OpenMP][Offloading] Fixed a crash caused by dereferencing nullptr

In function `DeviceTy::getTargetPointer`, `Entry` could be `nullptr` because of
zero length array section. We need to check if it is a valid iterator before
using it.

Reviewed By: ronlieb

Differential Revision: https://reviews.llvm.org/D116716

show more ...

[OpenMP][Offloading] Fixed data race in libomptarget caused by async data movement

The async data movement can cause data race if the target supports it.
Details can be found in [1]. This patch trie

[OpenMP][Offloading] Fixed data race in libomptarget caused by async data movement

The async data movement can cause data race if the target supports it.
Details can be found in [1]. This patch tries to fix this problem by attaching
an event to the entry of data mapping table. Here are the details.

For each issued data movement, a new event is generated and returned to `libomptarget`
by calling `createEvent`. The event will be attached to the corresponding mapping table
entry.

For each data mapping lookup, if there is no need for a data movement, the
attached event has to be inserted into the queue to gaurantee that all following
operations in the queue can only be executed if the event is fulfilled.

This design is to avoid synchronization on host side.

Note that we are using CUDA terminolofy here. Similar mechanism is assumped to
be supported by another targets. Even if the target doesn't support it, it can
be easily implemented in the following fall back way:
- `Event` can be any kind of flag that has at least two status, 0 and 1.
- `waitEvent` can directly busy loop if `Event` is still 0.

My local test shows that `bug49334.cpp` can pass.

Reference:
[1] https://bugs.llvm.org/show_bug.cgi?id=49940

Reviewed By: grokos, JonChesterfield, ye-luo

Differential Revision: https://reviews.llvm.org/D104418

show more ...

Revert "[OpenMP] Avoid costly shadow map traversals whenever possible"

This reverts commit 7c8f4e7b85ed98497f37571d72609f39a8eed447.
Fails a few OpenMP tests, causes a few updates to segfault.

[OpenMP] Avoid costly shadow map traversals whenever possible

In the OpenMC app we saw `omp target update` spending an awful lot of
time in the shadow map traversal without ever doing any update the

[OpenMP] Avoid costly shadow map traversals whenever possible

In the OpenMC app we saw `omp target update` spending an awful lot of
time in the shadow map traversal without ever doing any update there.
There are two cases that allow us to avoid the traversal completely.
The simplest thing is that small updates cannot (reasonably) contain
an attached pointer part. The other case requires to track in the
mapping table if an entry might contain an attached pointer as part.
Given that we have a single location shadow map entries are created,
the latter is actually fairly easy as well.

Reviewed By: grokos

Differential Revision: https://reviews.llvm.org/D113124

show more ...

[OpenMP][libomptarget] Change device vector elements to unique_ptr type

Using std::vector<DeviceTy> requires implementing copy constructor and copied assign operator for DeviceTy.
Indeed DeviceTy sh

[OpenMP][libomptarget] Change device vector elements to unique_ptr type

Using std::vector<DeviceTy> requires implementing copy constructor and copied assign operator for DeviceTy.
Indeed DeviceTy should never be copied. After changing to std::vector<std::unique_ptr<DeviceTy>>,
All the unsafe copy constructor and copy assign operator implementations can be removed.
Compilers mark them deleted due to mutex or underlying objects and this is the desired behavior.

Differential Revision: https://reviews.llvm.org/D109276

show more ...

[OpenMP] Use IsHostPtr where needed for targetDataBegin

As discussed in D105990, without this patch, `targetDataBegin`
determines whether to transfer data (as opposed to assuming it's in
shared memo

[OpenMP] Use IsHostPtr where needed for targetDataBegin

As discussed in D105990, without this patch, `targetDataBegin`
determines whether to transfer data (as opposed to assuming it's in
shared memory) using the condition `!UseUSM || HasCloseModifier`.
However, this condition is broken if use of discrete memory was forced
by `omp_target_associate_ptr`. This patch extends
`unified_shared_memory/associate_ptr.c` to reveal this case, and it
fixes it using `!IsHostPtr` in `DeviceTy::getTargetPointer` to replace
this condition.

Reviewed By: grokos

Differential Revision: https://reviews.llvm.org/D107927

show more ...

[OpenMP] Use IsHostPtr where needed for targetDataEnd

As discussed in D105990, without this patch, `targetDataEnd`
determines whether to transfer data or delete a device mapping (as
opposed to assum

[OpenMP] Use IsHostPtr where needed for targetDataEnd

As discussed in D105990, without this patch, `targetDataEnd`
determines whether to transfer data or delete a device mapping (as
opposed to assuming it's in shared memory) using two different
conditions, each of which is broken for some cases:

1. `!(UNIFIED_SHARED_MEMORY && TgtPtrBegin == HstPtrBegin)`: The
broken case is rare: the device and host might happen to use the
same address for their mapped allocations. I don't know how to
write a test that's likely to reveal this case, but this patch does
fix it, as discussed below.
2. `!UNIFIED_SHARED_MEMORY || HasCloseModifier`: There are at least
two broken cases:
1. The `close` modifier might have been specified on an `omp
target enter data` but not the corresponding `omp target exit
data`, which thus might falsely assume a mapping is in shared
memory. The test `unified_shared_memory/close_enter_exit.c`
already has a missing deletion as a result, and this patch adds
a check for that. This patch also adds the new test
`close_member.c` to reveal a missing transfer and deletion.
2. Use of discrete memory might have been forced by
`omp_target_associate_ptr`, as in the test
`unified_shared_memory/api.c`. In the current `targetDataEnd`
implementation, this condition turns out not be used for this
case: because the reference count is infinite, a transfer is
possible only with an `always` modifier, and this condition is
never used in that case. To ensure it's never used for that
case in the future, this patch adds the test
`unified_shared_memory/associate_ptr.c`.

Fortunately, `DeviceTy::getTgtPtrBegin` already has a solution: it
reports whether the allocation was found in shared memory via the
variable `IsHostPtr`.

After this patch, `HasCloseModifier` is no longer used in
`targetDataEnd`, and I wonder if the `close` modifier is ever useful
on an `omp target data end`.

Reviewed By: grokos

Differential Revision: https://reviews.llvm.org/D107925

show more ...

[OpenMP][OpenACC] Implement `ompx_hold` map type modifier extension in runtime (2/2)

This patch implements OpenMP runtime support for an original OpenMP
extension we have developed to support OpenAC

[OpenMP][OpenACC] Implement `ompx_hold` map type modifier extension in runtime (2/2)

This patch implements OpenMP runtime support for an original OpenMP
extension we have developed to support OpenACC: the `ompx_hold` map
type modifier. The previous patch in this series, D106509, implements
Clang support and documents the new functionality in detail.

Reviewed By: grokos

Differential Revision: https://reviews.llvm.org/D106510

show more ...

[OpenMP][Offloading] Add support for event related interfaces

This patch adds the support form event related interfaces, which will be used
later to fix data race. See D104418 for more details.

Rev

[OpenMP][Offloading] Add support for event related interfaces

This patch adds the support form event related interfaces, which will be used
later to fix data race. See D104418 for more details.

Reviewed By: jdoerfert, ye-luo

Differential Revision: https://reviews.llvm.org/D108528

show more ...

[OpenMP][Libomptarget] Adding `print_device_info` to RTL and `omptarget`

This patch introduces a function in the device's plugin to print the
device information. This patch relates to another patch

[OpenMP][Libomptarget] Adding `print_device_info` to RTL and `omptarget`

This patch introduces a function in the device's plugin to print the
device information. This patch relates to another patch that introduces
a CLI tool to obtain the device information from the omplibrary directly.
It is inspired by PGI's pgaccelinfo.

The modifications are as follows:
1. Introduce the optional `void __tgt_rtl_print_device_info(RTLdevID)` function into the RTL.
2. Introduce the `bool __tgt_print_device_info(devID)` function into `omptarget` interface. Returns false if the RTL is not implemented
3. Added `bool printDeviceInfo(RTLDevID)` to the `DeviceTy`
4. Implement the `__tgt_rtl_print_device_info` for CUDA. Added additional CUDA Runtime calls.

Reviewed By: jdoerfert

Differential Revision: https://reviews.llvm.org/D106751

show more ...

[libomptarget][nfc] Squash unused variable warning

Suppress only current warning on openmp-clang-x86_64-linux-debian

Reviewed By: jdoerfert

Differential Revision: https://reviews.llvm.org/D106777

[OpenMP][Offloading] Fix data race in data mapping by using two locks

This patch tries to partially fix one of the two data race issues reported in
[1] by introducing a per-entry mutex. Additional d

[OpenMP][Offloading] Fix data race in data mapping by using two locks

This patch tries to partially fix one of the two data race issues reported in
[1] by introducing a per-entry mutex. Additional discussion can also be found in
D104418, which will also be refined to fix another data race problem.

Here is how it works. Like before, `DataMapMtx` is still being used for mapping
table lookup and update. In any case, we will get a table entry. If we need to
make a data transfer (update the data on the device), we need to lock the entry
right before releasing `DataMapMtx`, and the issue of data transfer should be
after releasing `DataMapMtx`, and the entry is unlocked afterwards. This can
guarantee that: 1) issue of data movement is not in critical region, which will
not affect performance too much, and also will not affect other threads that don't
touch the same entry; 2) if another thread accesses the same entry, the state of
data movement is consistent (which requires that a thread must first get the
update lock before getting data movement information).

For a target that doesn't support async data transfer, issue of data movement is
data transfer. This two-lock design can potentially improve concurrency compared
with the design that guards data movement with `DataMapMtx` as well. For a target
that supports async data movement, we could simply attach the event between the
issue of data movement and unlock the entry. For a thread that wants to get the
event, it must first get the lock. This can also get rid of the busy wait until
the event pointer is valid.

Reference:
[1] https://bugs.llvm.org/show_bug.cgi?id=49940

Reviewed By: grokos

Differential Revision: https://reviews.llvm.org/D104555

show more ...

[OpenMP] Avoid checking parent reference count in targetDataBegin

This patch is an attempt to do for `targetDataBegin` what D104924 does
for `targetDataEnd`:

* Eliminates a lock/unlock of the data

[OpenMP] Avoid checking parent reference count in targetDataBegin

This patch is an attempt to do for `targetDataBegin` what D104924 does
for `targetDataEnd`:

* Eliminates a lock/unlock of the data mapping table.
* Clarifies the logic that determines whether a struct member's
host-to-device transfer occurs. The old logic, which checks the
parent struct's reference count, is a leftover from back when we had
a different map interface (as pointed out at
<https://reviews.llvm.org/D104924#2846972>).

Additionally, it eliminates the `DeviceTy::getMapEntryRefCnt`, which
is no longer used after this patch.

While D104924 does not change the computation of `IsLast`, I found I
needed to change the computation of `IsNew` for this patch. As far as
I can tell, the change is correct, and this patch does not cause any
additional `openmp` tests to fail. However, I'm not sure I've thought
of all use cases. Please advise.

Reviewed By: jdoerfert, jhuber6, protze.joachim, tianshilei1992, grokos, RaviNarayanaswamy

Differential Revision: https://reviews.llvm.org/D105121

show more ...

[OpenMP][Offloading] Refined return value of `DeviceTy::getOrAllocTgtPtr`

`DeviceTy::getOrAllocTgtPtr` just returns a target pointer. In addition,
two bool values (`IsNew` and `IsHostPtr`) are passe

[OpenMP][Offloading] Refined return value of `DeviceTy::getOrAllocTgtPtr`

`DeviceTy::getOrAllocTgtPtr` just returns a target pointer. In addition,
two bool values (`IsNew` and `IsHostPtr`) are passed by reference to make the
change in the function available in callee.

In this patch, a struct, which contains the target pointer, two flags, and an
iterator to the map table entry corresponding to the queried host pointer, will
be returned. In addition to make the logic clearer regarding the two bool values,
this paves the way for the next patch to fix the data race in `bug49334.cpp` by
attaching an event to the map table entry (and that's why we need the iterator).

Reviewed By: grokos

Differential Revision: https://reviews.llvm.org/D104382

show more ...