Rather than try to collect all `LastCell` types inside of a single
header and make an uber type, have each cell locator define its own cell
locator type and use that.
The `Variant` class was missing a way to check the type. You could do it
indirectly using `variant.GetIndex() == variant.GetIndexOf<T>()`, but
having this convenience function is more clear.
`Variant::CastAndCall` was using the C++11 style for an `auto` return
where the return type was specified with a `->` that got the `decltype`
of the return value of the functor. This was used as part of SFINAE to
pick whether you needed the const or non-const version.
However, this was causing a problem with functions that got an error
when deducing the return type for that. This was particularly
problematic for lambda functions. For example, say you have the
following simple `CastAndCall`.
```cpp
variant.CastAndCall([](auto& x){ ++x; });
```
To determine the return type of the lambda (`void`), the function has to
be compiled. But when it is compiled with a const type, which happens
when deducing the const version of `CastAndCall`, you get a compile
error. This error is not considered a substitution error (hence SFINAE),
it is an outright error. So you get a compile error just trying to
deduce the type.
The solution was to move to the C++14 version of an auto return type. In
this case, the return type is no longer important for SFINAE and is
delayed until the function is actually compiled with the specific
template parameters. This would be a problem if the const version of
`CastAndCall` was used when the non-const version was needed. But now
both versions will pass SFINAE and thus the non-const version will be
chosen as long as the `Variant` object itself is non-const. If the
`Variant` object itself is const, then that is in fact a legitimate
error, so a compile error is OK.
One thing I find wierd is that `CastAndCall` still has a `noexcept`
expression that will likewise cause a compile error in this case.
However, it is still working. I _think_ the difference is that
`noexcept` is not used to determine template substitution/overloaded, so
is therefore ignored until the function is actually compiled.
There was a typo in the declaration of the `CastAndCall` for a non-const
`Variant`. When determining the `noexcept` status of the function being
called, it was passing in a const reference instead of a regular
reference, which is what is actually passed to the function. This
potentially causes the function call to not match and fail to compile.
There was a bug where if you attempted to copy a variant that was not
valid (i.e. did not hold an object), a seg fault could happen. This has
been changed to set the target variant to also be invalid.
We have been doing a better job at hiding device code (and moving code
into libraries). Smoke out source that no longer needs to be compiled by
device compilers.
Previously, if you called `Get` on a `Variant` with a type that is not
in the list of types supported by the `Variant`, that would attempt to
look up the type at index `-1` and could spin the compiler into an
endless loop.
Instead, check for the case where you are attempting to get a type from
the `Variant` not listed in its templat arguments. In this case, instead
of producing a compiler error, produce a runtime error. Although this
increases the possibility that a bad compile path is being generated, it
simplifies creating templated code that produces cases we don't care
about.
Scheduling topology map workets for `CellSetExtrude` always worked, but
the there were indexing problems when a `Scatter` or a `Mask` was used.
This has been corrected, and now `Scatter`s and `Mask`s are supported on
topology maps on `CellSetExtrude`.
The reason why we did not support shared libraries when CUDA compiles
were on is that virtual methods require a special linking step to pull
together all virtual methods that might be called. I other words, you
cannot call a virtual CUDA method defined inside a library. This
requirement goes away when virtuals are removed.
Also removed the necessity of using seprable compilation with cuda.
Again, this is only needed when a CUDA function is defined in one
translation unit and used in another. Now we can enforce that all
translation units define their own CUDA functions.
Also, suppress warnings in cuda/internal/ExecutionPolicy.h
This is where we call the thrust algorithms. There must be some loop
where it, on some code path, calls back a host function. This must be in
an execution path that never happens. The thrust version has its own
suppress, but that does not seem to actually suppress the warning (it
just means that the warning does not tell you where the actual call is).
Get around the problem by suppressing the warnings in VTK-m.
Co-authored-by: Kenneth Moreland <morelandkd@ornl.gov>
Co-authored-by: Vicente Adolfo Bolea Sanchez <vicente.bolea@kitware.com>
Signed-off-by: Vicente Adolfo Bolea Sanchez <vicente.bolea@kitware.com>
The code in `vtkm/cont/Testing.h` now requires a library, which is not
built if testing is not built. Thus, the benchmarking code was giving a
compile error if benchmarking was on but testing was off.
Change the benchmarking to not rely on anything in the Testing
framework. This means using classes in `vtkm/source` instead of
`MakeTestData`. Also avoid using the `TestValue` defined for the tests.
(In one case, we have a simple replacement.) Also had to fix a problem
with a header file not defining everything it needed to compile.
Deprecate `VirtualObjectHandle` and all other classes that are used to
implement objects with virtual methods in the execution environment.
Additionally, the code is updated so that if the
`VTKm_NO_DEPRECATED_VIRTUAL` flag is set none of the code is compiled at
all. This opens us up to opportunities that do not work with virtual
methods such as backends that do not support virtual methods and dynamic
libraries for CUDA.
There appears to be a bug in CUDA 9.2 where if you have a class that
contains a struct that itself has to have padding in the middle for
alignment purposes and you then put that class in a union with other
classes, it seems like that padding can cause problems with other
objects in the union.
It always worked to trivially copy these classes, but the compiler did
not think so because copy constructors were defined. Change these
constructors to be default so that the compler can properly check
triviality.
Create a `VaraintUnion` that is an actual C++ `union` to store the data
in a `Variant`.
You may be asking yourself, why not just use an `std::aligned_union`
rather than a real union type? That was our first implementation, but
the problem is that the `std::aligned_union` reference needs to be
recast to the actual type. Typically you would do that with
`reinterpret_cast`. However, doing that leads to undefined behavior. The
C++ compiler assumes that 2 pointers of different types point to
different memory (even if it is clear that they are set to the same
address). That means optimizers can remove code because it "knows" that
data in one type cannot affect data in another type. To safely change
the type of an `std::aligned_union`, you really have to do an
`std::memcpy`. This is problematic for types that cannot be trivially
copied. Another problem is that we found that device compilers do not
optimize the memcpy as well as most CPU compilers. Likely, memcpy is
used much less frequently on GPU devices.
`std::is_trivial` is part of the C++14 specification. However, we have
encountered multiple compilers that purport to implement C++14 but do
not implement `std::is_trivial` and the like checks correctly.
To avoid such issues, only use `std::is_trivial` on compilers that we
have tested to support it.
`PointLocator`s have changed from being virtual objects to being trivial
objects. Part of this change means that when a worklet gets the
execution object for a point locator, it gets the actual object (or a
reference to it) instead of a pointer to an object. This means that the
new code uses the `.` operator to access the locator's features instead
of the `->` operator.
To support code still using the deprecated functionality, added a
specific `->` operator to the locator execution object to make it behave
as if it were a pointer. However, this operator is marked deprecated to
warn the user that they should modify their code to use the `.` instead.
Deprecated the `CellLocator` class and made all methods of the
other `CellLocator` classes non-virtual. General locators can
still use the `CellLocatorGeneral` class, but this class now
only works with a predefined set of locators. (The functionality
to provide a function to select a locator has been removed.)
0797359c5 Make ExecutionWholeArray objects not depend on device type
0bee74438 Support DeviceAdapterId in deprecated ArrayHandle
Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Nick Thompson <nathompson7@protonmail.com>
Merge-request: !2405
With recent changes to `Arrayhandle`, the type for the associated array
portal is now the same across all devices. This means that almost all
exec objects no longer need to be specialized on the device types. Thus,
clean up the whole array exec objects to no longer need to be templated
on device.
With recent changes to `ArrayHandle`, the type for the associated array
portal is now the same across all devices. This means that almost all
exec objects no longer need to be specialized on the device types. Thus,
clean up the locator exec objects to no longer need to be templated on
device.
Somewhere during this edit I removed a header file that didn't strictly
need to be there. This caused me to have to add
```cpp
```
in several places in the code.
The newer version of `ArrayHandle` no longer supports different types of
portals for different devices. Thus, the `ReadPortalType` and
`WritePortalType` are sufficient for all types of portals across all
devices.
This significantly simplifies supporting execution objects on devices,
and thus this change also includes many changes to various execution
objects to remove their dependence on the device adapter tag.
Some (but not all) versions of visual studio seem to have a bug that
causes a syntax error when using the deprecated attribute in a templated
constructor.
The actual code for AtomicArrayExecutionObject does not need to be
specialized by the device. The functionality is implemented by calling
the vtkm::Atomic* methods, which are properly implemented on each
device.
Some of the special `ArrayHandle`s require specialized versions of
`vtkm::exec::arg::Fetch`. The specializations were not put in the
respective vtkm/exec/arg/Fetch*.h header files because the definition of
the `ArrayHandle`s was not available there. The implementation was in
the ArrayHandle*.h files, but it is hard to find the specialization
there.
Instead, make a secondary header file in vtkm/exec/arg that implements
the Fetch specialization and include it from the ArrayHandle*.h file.
That way, the basic Fetch does not have to include odd `ArrayHandle`
types but the `Fetch` implemenations are still all located together.
The `Variant` class has separate implementations for its move and copy
constructors/assignment operators depending on whether the classes it
holds can be trivially moved. If the objects are trivial, Variant is
trivial as well. However, in the case where the objects are not trivial,
special construction and copying needs to be done.
Previously, the non-trivial `Variant` defined a move constructor that
did a byte copy of the contained object and reset the right hand side
object so that it did not attempt to destroy the object. That usually
works because it guarantees that only one version of the `Variant` will
attempt to destroy the object and its resources should be cleaned up
correctly.
But C++ is a funny language that lets you do weird things. Turns out
there are cases where moving the location of memory for an object
without calling the proper copy method can invalidate the object. For
example, if the object holds a pointer to one of its own members, that
pointer will become invalid. Also, if it points to something that points
back, then the object will need to update those pointers when it is
moved. GCC's version of `std::string` seems to be a type like this.
Solve the problem by simply deleting the move constructors. The copy
constructors and destructor will be called instead to properly manage
the object. A test for these conditions is added to `UnitTestVariant`.