Update complex interface

[jdemel]

This is a draft.

API break: This work breaks the API on purpose.

Mostly, it is a try to fix these issues with C vs. C++ complex types. We rely on undefined behavior. This is supposed to end and be better.

Apparently, we hit a bunch of issues:

• complex is considered a "niche"
• complex.h and std::complex seem to be explicitly binary incompatible. At least they're allowed to.
• Issues multiply as soon as we consider non float types. e.g. char _Complex.
• Probably more issues we haven't even encountered yet.

EDIT Fixes #442 Fixes #572

Current state:

• C Interface is updated.
• C++ must use C interface and wrap around it.
• C++ usage is somehow less convenient at the moment. Wrappers missing.
• PowerPC issues are fixed.
• CI passes on Linux x86 and arm.
• CI fails on Windows and MacOS.

EDIT Since we want to support compilers that are difficult to support with complex C (i.e. complex C support is missing), we ended up with the current API that basically relies on Undefined Behavior (UB).

[marcusmueller]

you can't fix this without an API break, so break API in a visible, and useful manner, and completely remove the same-name C++ wrapper; instead of

void volk_32fc_x2_dot_prod_32fc(lv_32fc_t*, const lv_32fc_t*, const lv_32fc_t*, unsigned int)

a C++ consumer should be calling

volk::dot_prod(const std::vector<std::complex<float>>&,const std::vector<std::complex<float>>&, std::vector<std::complex<float>>&)

or

void volk::dot_prod(const std::complex<float>*, const std::complex<float>*, std::complex<float>*, unsigned int)

we should be generating these wrappers automatically.

Ideally, these wrappers wouldn't just be C++ functions, but static operator(...) members of a class (per kernel), so that we can later add functionality like querying kernels what their favorite granularity is, which protokernels exist and so on.

[jdemel]

Thanks for your suggestions. I want to update the interface and that'll break the current API.

The C++ interface should require volk::vector because we can assume they're aligned. If we'd use any kind of vector, the C++ interface doesn't need a separate vector length parameter anymore.

However, at the moment, I'm stuck with a more trivial problem. For any interface, we'd need to be able to call a C kernel with an lv_32fc_t datatype. Right now, I'm unable to compile any C++ as long as I try to use a C typedef. The issue is: How do I convince my C++ compiler to pick up C compelx data type definitions?

[jdemel]

At the moment I assume, the updated C API is in good shape. The MSVC error messages in our CI are currently not helpful. This needs further investigation. The MacOS error is probably due to some __VOLK_DECL_BEGIN specifics. We probably want to drop the && (__GNUC__) condition.

The C++ API update is in a more experimental state. It works with 2 examples.

C++ Interface requirements

1. Make C++ STL types usable std::vector, std::complex.
2. Make aligned vectors aka volk::vector usable.
3. Allow call-by-pointer for GR buffer interface usage etc.

These requirements result in at least 3 functions. We might want to think about fancy new C++ features e.g. concepts to consolidate these.

More considerations: Do we want to expose more info? e.g.

• explicit function pointers.
  • Especially: Make these explicitly callable?
• adopt a volk::dot_prod::operator like interface.
• add option to obtain available implementations for a kernel in C++?
• Add an explicit volk.hh, or volkpp.h, or volkcpp.h header file for the C++ API. That'd include volk_alloc.hh.

[jdemel]

At this point, everything compiles and and all tests pass on most systems with most compilers. It seems like there are 2 notable exceptions: AppleClang and MSVC. In both cases, I need help. @michaelld could you have a look at the MacOS issue? @ryanvolz would you be able to help out? Or do you know whom we could ask for help?

[michaelld]

@jdemel the basic concept of the extern "C" { (__VOLK_DECL_BEGIN) is to use it only on local code, not on #include <>s. Thus in include/volk/volk_complex.h, change the start to:

#include <volk/volk_common.h>
#include <complex.h>

__VOLK_DECL_BEGIN

and then end with

#endif /* __GNUC__ */

__VOLK_DECL_END

#endif /* INCLUDE_VOLK_COMPLEX_H */

And for tmpl/volk_typedefs.tmpl.h:

#include <inttypes.h>
#include <volk/volk_common.h>
#include <volk/volk_complex.h>

__VOLK_DECL_BEGIN

%for kern in kernels:
typedef void (*${kern.pname})(${kern.arglist_types});
%endfor

__VOLK_DECL_END

with these changes, this PR builds on macOS for me. These changes should be broadly correct for modern compilers.

[jdemel]

Thanks @michaelld MacOS passes CI now too.

What are your thoughts for the C++ interface?

[michaelld]

Thanks @michaelld MacOS passes CI now too.

Yay! I'll look at the Ubuntu failures too & see if I can work out what's going on there.

What are your thoughts for the C++ interface?

I like it, in general. I think it's important for us to offer this interface, since there is a lot of complex math that can be accelerated by appropriately-designed kernels.

[michaelld]

Ah ,... the Ubuntu ones are still working. Looking positive right now, doing make test. Not sure if the Windows ones are going or dead; I can't really help there anyway ...

[ryanvolz]

The Windows failure seems consistent with https://docs.microsoft.com/en-us/cpp/c-runtime-library/complex-math-support?view=msvc-160. Namely, that _Complex is not defined with MSVC. You could do some ifdefs and use _FComplex and friends instead, but that looks like it will only work for float and double. I don't have a lead on what to do for the rest of it.

[jdemel]

My first idea was to browse through: MSVC STL headers I found a reference to complex.h but not the C header file complex.h. If you happen to know a place where we can find it, that'd be very useful. MSVC has a generic std::complex class that might be able to do std::complex<int> etc.

Besides, reading through these MSVC specifics, they basically, once again, state: MSVC supports C89-only and only select later extensions. Thus, requiring C11 excludes MSVC.

If we can't fix it, we might be forced to revert to define our own custom structure types. Obviously, that'd be a very unpleasant prospect.

[ryanvolz]

If you happen to know a place where we can find it, that'd be very useful.

It's distributed as part of the Universal C Runtime, which comes in the Windows SDK. I haven't found a way of viewing just the header without installing the whole thing on a Windows machine, but that's a place to start.

[jdemel]

@ryanvolz Thanks for the hints where to search! From MSVC complex.h

typedef struct _C_float_complex
{
    float _Val[2];
} _C_float_complex;

typedef _C_float_complex   _Fcomplex;

Thus, we should be able to do things like:

typedef _Fcomplex lv_32fc_t;
typedef _Dcomplex lv_64fc_t;

However, we might be forced to define these structs manually.

typedef char _Complex lv_8sc_t;
typedef short _Complex lv_16sc_t;
typedef long _Complex lv_32sc_t;
typedef long long _Complex lv_64sc_t;

In that case, we might be forced to overload complex math functions as well. Thus, whenever one adds a function that uses these types and some complex math functions, we might need to implement these for a new type. However, I assume we'd just do the convert to double and back again thing. It is probably inefficient but should only be required for generic kernels.

[jdemel]

I tried to define

typedef _Fcomplex lv_32fc_t;

for MSVC, but the CI tells me

error C4430: missing type specifier - int assumed. Note: C++ does not support default-int

There's something wrong. At this point, I need help from someone with such an environment.

[ryanvolz]

I have figured out the initial problem: Microsoft's <complex.h> simply includes <ccomplex> if it is included in a C++ compilation, and we have been compiling volk as C++ with MSVC in order to make use of <ccomplex> previously. This diff gets things moving:

diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt
index fb9a8f9..4f207d0 100644
--- a/lib/CMakeLists.txt
+++ b/lib/CMakeLists.txt
@@ -496,8 +496,6 @@ if(MSVC)
     #add compatibility includes for stdint types
     include_directories(${PROJECT_SOURCE_DIR}/cmake/msvc)
     add_definitions(-DHAVE_CONFIG_H)
-    #compile the sources as C++ due to the lack of complex.h under MSVC
-    set_source_files_properties(${volk_sources} PROPERTIES LANGUAGE CXX)
 endif()
 
 #Create an object to reference Volk source and object files.

Now I get the following error about multiplication not being defined for the complex types:

[33/54] Building C object lib\CMakeFiles\volk_obj.dir\volk_machine_generic.c.obj
FAILED: lib/CMakeFiles/volk_obj.dir/volk_machine_generic.c.obj
C:\PROGRA~2\MICROS~2\2019\BUILDT~1\VC\Tools\MSVC\1416~1.270\bin\HostX64\x64\cl.exe  /nologo -DBOOST_ALL_DYN_LINK -DHAVE_CONFIG_H -DHAVE_FENV_H -DHAVE_INTRIN_H -D_GLIBCXX_USE_CXX11_ABI=1 -D_USE_MATH_DEFINES -IZ:\repos\volk\cmake\msvc -IZ:\repos\volk\build\include -IZ:\repos\volk\include -IZ:\repos\volk\kernels -IZ:\repos\volk\build\lib -IZ:\repos\volk\lib -IZ:\repos\volk\cpu_features\include /DWIN32 /D_WINDOWS /W3 -Wall /MD /O2 /Ob2 /DNDEBUG /W1 /wo4309 /wd4752 /wo4273 /wo4838 /showIncludes /Folib\CMakeFiles\volk_obj.dir\volk_machine_generic.c.obj /Fdlib\CMakeFiles\volk_obj.dir\ /FS -c Z:\repos\volk\build\lib\volk_machine_generic.c
cl : Command line warning D9025 : overriding '/W3' with '/W1'
Z:\repos\volk\kernels\volk/volk_16i_32fc_dot_prod_32fc.h(84): error C2088: '*': illegal for struct

I don't know how far down this rabbit hole you want to go, but this seems to point to the next step.

[jdemel]

I have figured out the initial problem: Microsoft's <complex.h> simply includes <ccomplex> if it is included in a C++ compilation, and we have been compiling volk as C++ with MSVC in order to make use of <ccomplex> previously. This diff gets

Thanks for having a look at it.

[33/54] Building C object lib\CMakeFiles\volk_obj.dir\volk_machine_generic.c.obj
FAILED: lib/CMakeFiles/volk_obj.dir/volk_machine_generic.c.obj
C:\PROGRA~2\MICROS~2\2019\BUILDT~1\VC\Tools\MSVC\1416~1.270\bin\HostX64\x64\cl.exe  /nologo -DBOOST_ALL_DYN_LINK -DHAVE_CONFIG_H -DHAVE_FENV_H -DHAVE_INTRIN_H -D_GLIBCXX_USE_CXX11_ABI=1 -D_USE_MATH_DEFINES -IZ:\repos\volk\cmake\msvc -IZ:\repos\volk\build\include -IZ:\repos\volk\include -IZ:\repos\volk\kernels -IZ:\repos\volk\build\lib -IZ:\repos\volk\lib -IZ:\repos\volk\cpu_features\include /DWIN32 /D_WINDOWS /W3 -Wall /MD /O2 /Ob2 /DNDEBUG /W1 /wo4309 /wd4752 /wo4273 /wo4838 /showIncludes /Folib\CMakeFiles\volk_obj.dir\volk_machine_generic.c.obj /Fdlib\CMakeFiles\volk_obj.dir\ /FS -c Z:\repos\volk\build\lib\volk_machine_generic.c
cl : Command line warning D9025 : overriding '/W3' with '/W1'
Z:\repos\volk\kernels\volk/volk_16i_32fc_dot_prod_32fc.h(84): error C2088: '*': illegal for struct

I don't know how far down this rabbit hole you want to go, but this seems to point to the next step.

So MSVC does not support any of the things we need. If we want MSVC to compile VOLK and avoid a C and C++ mix, we basically need to re-implement complex.h. I don't see how this might be a viable solution.

Currently, my idea would be: Use Clang to compile VOLK with MSVC ABI compat and use MSVC to compile tests, volk_profile etc. Since VOLK is effectively a C++ library for MSVC users, that wouldn't exclude anyone. In case of MSVC, we can still use our "rely on undefined behavior" approach because it works on x86 and ARM. I have no idea if this approach is viable. However, I suppose it is a saner approach than: re-implement complex.h.

I'm open to suggestions. Also, the current state with a widely used C compiler that is effectively C89 with some random improvements is really unsatisfactory.

[ryanvolz]

Currently, my idea would be: Use Clang to compile VOLK with MSVC ABI compat and use MSVC to compile tests, volk_profile etc.

From my conda perspective, using Clang to compile VOLK shouldn't be a problem, although I haven't tried it and I am not aware of the full implications of doing that for downstream consumers (i.e. GNU Radio).

[jdemel]

I don't have any experience with that approach. I assume, if we find a way to compile VOLK with Clang on Windows and it is possible to build GNU Radio against it we're good to go.

[jdemel]

With VOLK 3.1 we went the temporary route to pass complex values by pointer exclusively. While this is not the nicest solution, it works.