Commit Graph

17 Commits

Author SHA1 Message Date
Kenneth Moreland
d1dba170e8 Do not use volatile when calling CUDA atomicCAS
Although it makes sense to assume a pointer is `volatile` when doing an
atomic operation, the arguments to the `atomicCAS` overloads take
regular pointers. The overload resolution can fail if you use the
`volatile` keyword.
2021-03-25 10:11:44 -06:00
Li-Ta Lo
b590a8ebb2 Add floating point atomics 2021-03-11 08:19:51 -07:00
Robert Maynard
c5efdb39ac vtkm/Atomic doesn't generate unused parameter warnings on MSVC 2021-01-11 09:01:48 -05:00
Kenneth Moreland
28ecf3636d Change interface of atomic compare and swap
The old atomic compare and swap operations (`vtkm::AtomicCompareAndSwap`
and `vtkm::exec::AtomicArrayExecutionObject::CompareAndSwap`) had an
order of arguments that was confusing. The order of the arguments was
shared pointer (or index), desired value, expected value. Most people
probably assume expected value comes before desired value. And this
order conflicts with the order in the `std` methods, GCC atomics, and
Kokkos.

Change the interface of atomic operations to be patterned off the
`std::atomic_compare_exchange` and `std::atomic<T>::compare_exchange`
methods. First, these methods have a more intuitive order of parameters
(shared pointer, expected, desired). Second, rather than take a value
for the expected and return the actual old value, they take a pointer to
the expected value (or reference in `AtomicArrayExecutionObject`) and
modify this value in the case that it does not match the actual value.
This makes it harder to mix up the expected and desired parameters.
Also, because the methods return a bool indicating whether the value was
changed, there is an additional benefit that compare-exchange loops are
implemented easier.

For example, consider you want to apply the function `MyOp` on a
`sharedValue` atomically. With the old interface, you would have to do
something like this.

```cpp
T oldValue;
T newValue;
do
{
  oldValue = *sharedValue;
  newValue = MyOp(oldValue);
} while (vtkm::AtomicCompareAndSwap(sharedValue, newValue, oldValue) != oldValue);
```

With the new interface, this is simplfied to this.

```cpp
T oldValue = *sharedValue;
while (!vtkm::AtomicCompareExchange(sharedValue, &oldValue, MyOp(oldValue));
```
2020-10-20 08:39:22 -06:00
Robert Maynard
f04b12d9db vtkm/Atomic can now safely include kokkos when HIP is enabled 2020-10-20 09:24:05 -04:00
Robert Maynard
5fe6c36fa2 vtkm::AtomicLoad should only occur on non-const values
Previously vtk-m allowed users to issue atomic loads on constant
values which is problematic for the following reasons:

 - can be a source of undefined behavior
 - not supported by kokkos

This issue was detected when using kokkos HIP atomic implementation
2020-10-09 14:20:21 -04:00
Kitware Robot
cf0cdcf7d1 clang-format: reformat the repository with clang-format-9 2020-08-24 14:01:08 -04:00
Kenneth Moreland
6cbcb9f5d7 Fix behavior of Cuda AtomicLoad with SequentiallyConsistent
According to Allie Vacanti, a sequentially consistent load requires a
full fence on Cuda hardware to be conforming.

Also improved the documentation of `MemoryOrder` based on Allie's
suggestion.

Also removed the `Consume` memory order based on Allie's suggestion. It
is tricky to use correctly, and most implementations just regress to the
safer `Acquired` behavior anyway.
2020-08-20 17:53:47 -06:00
Kenneth Moreland
7573d4ed57 Fix compiler warnings 2020-08-20 17:08:36 -06:00
Kenneth Moreland
147dd24d01 Remove ARM intrinsics in MSVC 2020-08-20 17:08:36 -06:00
Kenneth Moreland
2229c22f40 Avoid invalid Kokkos atomic calls 2020-08-20 17:08:21 -06:00
Kenneth Moreland
3b147878f5 Always use our implementation of Cuda atomics
Previously, if Kokkos was enabled we always used Kokkos atomics.
However, if a user, for some reason, compiled with a version of Kokkos
that was _not_ compiled for Cuda and we turned on our own Cuda device
adapter, that would cause a problem. The old code assumed Kokkos would
create the Cuda version of the atomics, but it would not. Thus, there
would be no atomics for Cuda.

Resolved this problem by switching the order in which we try to define
atomics. Use our version of atomics whenever compiling for a Cuda
device. Otherwise, try to compile the Kokkos version (and if that is not
available, use ours as before.
2020-08-20 14:20:32 -06:00
Kenneth Moreland
9e6fe8fb66 Add memory order semantics to atomic functions
To ensure correctness of an atomic function, it is often necessary to
force a compiler to order operations correctly. However, doing so may
slow things down, so it is helpful to relax these constraints if they
are not necessary. Add the ability to select the correct memory order
constraints in the atomic functions.
2020-08-20 13:40:44 -06:00
Kenneth Moreland
d2ac4b860c Be more careful in casting with Atomic functions
Previously, the atomic functions accepted any type as its operand and
then cast that to the storage type. That could cause some rather
unexpected behavior. For example, casting a floating point number to an
integer might not give you the behavior as expected. So that behavior as
been removed and now the operand has to match the pointer.

However, all the currently supported atomics are unsigned, and there are
many reasons that it might be easier to use signed as operands. (For
example, C literals are signed.) Thus, a second condition that allows
the sign to be swapped has been added so that you don't get annoying
signed/unsigned conversion warnings.
2020-08-20 13:40:44 -06:00
Kenneth Moreland
13056b3af5 Deprecate AtomicInterfaceControl and AtomicInterfaceExecution
Now that we have the functions in `vtkm/Atomic.h`, we can deprecate (and
eventually remove) the more cumbersome classes `AtomicInterfaceControl`
and `AtomicInterfaceExecution`.

Also reversed the order of the `expected` and `desired` parameters of
`vtkm::AtomicCompareAndSwap`. I think the former order makes more sense
and matches more other implementations (such as `std::atomic` and the
GCC `__atomic` built ins). However, there are still some non-deprecated
classes with similar methods that cannot easily be switched. Thus, it's
better to be inconsistent with most other libraries and consistent with
ourself than to be inconsitent with ourself.
2020-08-20 13:40:44 -06:00
Kenneth Moreland
82d6ca9854 Allow Kokkos atomic functions to work without cuda device compiler
If the Kokkos device is enabled, we use the Kokkos atomic functions for
implementation since Kokkos has already ported all of these to each
device.

If Kokkos is compiled with CUDA, then the functions are marked with
`__device__` and `__host__`. Makes sense right?

Unless we are trying to use the atomic functions on host code compiled
only for the host. In that case, modifiers like `__device__` will cause
compiler errors.

So we want to disable Kokkos from using `__device__`, but only if CUDA
is enabled and we are not using the CUDA compiler. Had to hack things up
to get that to work.
2020-08-20 13:40:44 -06:00
Kenneth Moreland
ebbebd7369 Add atomic free functions
Previously, all atomic functions were stored in classes named
`AtomicInterfaceControl` and `AtomicInterfaceExecution`, which required
you to know at compile time which device was using the methods. That in
turn means that anything using an atomic needed to be templated on the
device it is running on.

That can be a big hassle (and is problematic for some code structure).
Instead, these methods are moved to free functions in the `vtkm`
namespace. These functions operate like those in `Math.h`. Using
compiler directives, an appropriate version of the function is compiled
for the current device the compiler is using.
2020-08-20 13:40:43 -06:00