With the major revision 2.0 of VTK-m, many items previously marked as
deprecated were removed. If updating to a new version of VTK-m, it is
recommended to first update to VTK-m 1.9, which will include the deprecated
features but provide warnings (with the right compiler) that will point to
the replacement code. Once the deprecations have been fixed, updating to
2.0 should be smoother.
For several versions, VTK-m has had a `Variant` templated class. This acts
like a templated union where the object will store one of a list of types
specified as the template arguments. (There are actually 2 versions for the
control and execution environments, respectively.)
Because this is a complex class that required several iterations to work
through performance and compiler issues, `Variant` was placed in the
`internal` namespace to avoid complications with backward compatibility.
However, the class has been stable for a while, so let us expose this
helpful tool for wider use.
Several revisions ago, the ability to use virtual methods in the
execution environment was deprecated. Completely remove this
functionality for the VTK-m 2.0 release.
This mechanism sets up CMake variables that allow a user to select which
modules/libraries to create. Dependencies will be tracked down to ensure
that all of a module's dependencies are also enabled.
The modules are also arranged into groups.
Groups allow you to set the enable flag for a group of modules at once.
Thus, if you have several modules that are likely to be used together,
you can create a group for them.
This can be handy in converting user-friendly CMake options (such as
`VTKm_ENABLE_RENDERING`) to the modules that enable that by pointing to
the appropriate group.
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.
This commit address issue #713 and removes the need to pass information
about the spatial decomposition (block origins, block sizes, block index
and blocks per dimension) to the contour tree filter. Block origin
information is added to CellSetStructured (as GlobalPointSize) and the
distributed contour tree filter will get this information from
CellSetStructured instead of expecting it as parameters to the
constructor. Information about blocks per dimension and block indices
are computed from the information in CellSetStructure albeit requiring
global communication across all ranks. To avoid this communication cost,
the caller of the filter may explicitly specify this information via the
SetBlockIndices() method.
`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.
The current implementation of the AMD HIP compiler adds default compiler
flags to attempt to inline everything possible. This is wrecking havoc
with the `Variant::CastAndCall` function, whose implementation has one
or more large switch statements with each case calling a different
potentially inline function. In some parts of the VTK-m code, this is
dragging the compilation on for days as it tries to resolve a
`Variant::CastAndCall` within a `Variant::CastAndCall`. This will
probably need to be addressed in the compiler, but meanwhile we will
force the inlining to be turned off for the function called by
`Variant::CastAndCall`. It is unclear if this will result in some extra
runtime overhead, but the change is worth it to get reasonable compile
times.
Thanks to Nick Curtis for tracking this down and providing the solution.
The previous implementation of `Variant`'s `CastAndCall` generated a
switch statement with 20 cases (plus a default) regardless of how many
types were handled by the `Variant` (with the excess doing nothing
useful). This reduced the amount of code, but caused the compiler to
have to build many more instructions (and optimize for them). This in
turn lead to large compile times and unnecessary large libraries/
executables.
This change makes a different function to use for `CastAndCall` so that
the number of cases in the switch matches exactly the number of types in
the `Variant`'s union.
Because the size of VariantImplDetail.h was getting large, I also
reduced the maximum expansions for the code. This does not seem to
negatively affect compile time, and I doubt it will have an noticible
difference in running time (when in release mode).
I also modified some other parts of this code to match the expansion
without making unnecessary defaults.
Recently, an instantiation method was added to the VTK-m configuration
files to set up a set of source files that compile instances of a template.
This allows the template instances to be compiled exactly once in separate
build files.
However, the implementation made the assumption that the instantiations
were happening for VTK-m filters. Now that the VTK-m filters are being
redesigned, this assumption is broken.
Thus, the instantiation code has been redesigned to be more general. It can
now be applied to code within the new filter structure. It can also be
applied anywhere else in the VTK-m source code.
In pretty much any practical circumstance, whenusing `ListAll` or
`ListAny`, you have a list of types on which you run some sort of
predicate on each item in the list to determine whether any or all of
the items match the predicate. To make this easier, add a second
argument to `ListAll` and `ListAny` to provide a predicate that will
automatically be added.
If no predicate is given, then the operation is run directly on the
list. This is implemented by just using an identity operation.
GCC 11 is having trouble compiling brigand.hpp at all, even before we
instantiate any templates. Since we no longer need it, let's get rid of
it. It was always placed in an internal namespace.
Add deprecation warnings to the code whenever someone uses brigand.hpp.
We are no longer supporting this header file, but we'll give code a
chance to transition off of it.
Also added some other deprecation warnings to other header files that
are themselves deprecated but only issued warnings if you used something
in it.
These new features to VTK-m lists allow you to compute a single value
from a list. `ListReduce` allows you to compute a value based on a
predicate. `ListAll` and `ListAny` use this feature to determine if all
or any of a list of `true_type` or `false_type` objects are true.
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.
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.
For some reason some versions of the CUDA compiler would return true for
`is_trivially_copyable` on a `VariantUnion` even when the types of the
union caused the copy constructor to get deleted.
Solve the problem by using `AllTriviallyCopyable` instead of directly
caling `is_trivially_copyable` on the union.
Nvcc was having troubles resolving the return type of the overloaded
function to get a value out of a `VariantUnion`. Replace the
implementation with a class with specializations. This is more verbose,
but easier on the compiler.
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.
I have run into strange problems with reference types sneaking into
templates. Reference types can cause weird errors when passing things
across hosts and devices, and it's best not to let them sneak into an
object like Varient where they become more difficult to check for.
There appears to be a bug in GCC where if you mixed C++ attributes like
`[[deprecated]]` with GNU attributes like `__attribute__(())`, you will
get a compile error. This can be a problem when using attributes to both
set the export (i.e. visibility) of an item and deprecating the same
item.
This problem has been encountered before. Commit
34b0bba84207a89e8fddfe62e7a1ff30b1b53a18 fixed the problem by using
`[[gnu::visibility()]]` instead of `__attribute__()` in the export
macros. This works fine for export macros defined by VTK-m headers such
as `VTKM_ALWAYS_EXPORT`. Unfortunately, we have little control over the
export macros that CMake automatically creates. Rather than rewrite the
CMake export code, we go the other way and use `__attribute__()` for all
exports _and_ the depreciated attribute.
7475c318b VTK-m now uses CMake's future HIP lang for Kokkos+HIP
Acked-by: Kitware Robot <kwrobot@kitware.com>
Acked-by: Sujin Philip <sujin.philip@kitware.com>
Merge-request: !2351
Many of the fancy `ArrayHandle`s are read-only and therefore connot
really create write portals. Likewise, many `ArrayHandle`s (both read-
only and read/write) have no way to resize themselves. In this case,
implementing the `CreateWritePortal` and `ResizeBuffers` methods in the
`Storage` class was troublesome. Mostly they just threw an exception,
but they also sometimes had to deal with cases where the behavior was
allowed.
To simplify code for developers, this introduces a pair of macros:
`VTKM_STORAGE_NO_RESIZE` and `VTKM_STORAGE_NO_WRITE_PORTAL`. These can
be declared in a `Storage` implementation when the storage has no viable
way to resize itself and create a write portal, respectively.
Having boilerplate code for these methods also helps work around
expected behavior for `ResizeBuffers`. `ResizeBuffers` should silently
work when resizing to the same size. Also `ResizeBuffers` should behave
well when resizing to 0 as that is what `ReleaseResources` does.
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`.
The `Variant` class is templated to hold objects of other types.
Depending on whether those objects of are meant to be used in the
control or execution side, the methods on `Variant` might need to be
declared with (or without) special modifiers. We can sometimes try to
compile the `Variant` methods for both host and device and ask the
device compiler to ignore incompatibilities, but that does not always
work.
To get around that, create two different implementations of `Variant`.
Their API and implementation is exactly the same except one declares its
methods with `VTKM_CONT` and the other its methods `VTKM_EXEC`.
