C API feedback

[bragadeesh]

Lets use this issue tracker to comment on C API design.

[b-sumner]

I'm having trouble with this part of the example:

// get pointers to enable read/write of data
status = rocfft_buffer_get_ptr(buffer_a, &raw_ptr_a);
status = rocfft_buffer_get_ptr(buffer_a, &raw_ptr_b);

// initialize input
...

The get_ptr return is something like a blocking clEnqueueMapBuffer call. How do you tell the API that you're done with the pointer and to migrate the data back to the device? Or if you prefer AMP concepts, the get_ptr return is something like an array_view, and there needs to be a call to synchronize() to copy it to the device.

[b-sumner]

Shouldn't there be a size_in_bytes or some other argument to buffer_create_with_ptr to enable some basic range checking?

[bragadeesh]

After thinking about this a bit more, with some more discussion/inputs from Kent and Timmy, here are my thoughts:

First, here are some of the assumptions.

1. Decision made to keep the rocFFT C header not dependent on HIP or HC C++ headers
2. Hide anything HIP/HC C++ related types through void pointers
3. C interface should be consumable by vanilla C programmers and from other languages such as Fortran, Python etc

Given this, I propose 2 approaches to managing device memory

---

Approach 1:

• rocFFT C interface should be used ONLY by programmer who is already developing with HIP or HC C++
• Have NO memory related APIs in the rocFFT header
• Whenever we need to get device memory from the user, we obtain it through void pointer (a value returned by call to hipMalloc in the application) in the library API.

In adition to rocFFT interface, we provide an exact replica of FFTW interface (which hides all GPU programming) to support other languages. Here, we manage all device memory within the library; the user only sees host memory regions with calls to fftw_malloc etc. Within the execute API function, we copy data from host to device, do compute immediately and write it back; there is performance implication with this though but we keep the semblance of an ordinary C library to the user

Approach 2:

• rocFFT C interface can be consumed by anyone (with or without the use of HIP or HC/C++ programming in the application)
• Must have memory related APIs in the library header; it should provide user these basic tasks: create/free device memory region, copy data from/to device
• Should work harmoniously with any mixing of hipMalloc and rocFFT malloc

Providing replica FFTW interface is not needed in this case, but can be done for convenience.

---

My current rocFFT header kind of followed approach 2, but I realize I have not provided all of the features. I only created entry points for allocation/freeing of device memory. There is no explicit api for copying of data between host and device. I assumed the application developer can always use HIP functions for these; but I realize this is not an option for other language programmers. I feel that if we are going to have a library abstraction layer that deals with memory management, then we should have it independent of rocFFT. Maybe a base/common library that other libs such as rocBLAS can use.

I am starting to favor approach 1. I am going to delete memory related APIs from rocFFT. I can document how device memory created with HIP or HC C++ need to be passed to the library. There would not be any type safety though, with void pointer arguments.

@b-sumner with regard to your question above, my idea with the function 'rocfft_buffer_get_ptr' is to give the pointer created by call to hipMalloc inside the library back to the user and it is a very simple non blocking call. This pointer would be useless for de-referencing or pointer arithmetic on the host side.

[kknox]

I am also an advocate of approach 1

rocFFT C interface should be used ONLY by programmer who is already developing with HIP or HC C++

I think this should just be framed to say that native C interfaces enable language wrappers. It can be used by anybody, and anybody can make a language wrapper if they so desire.

Whenever we need to get device memory from the user, we obtain it through void pointer (a value returned by call to hipMalloc in the application) in the library API

void* can come from mutliple sources: hipMalloc, hc::am_alloc and clSVMalloc(). So, void* should enable a very flexible, non-typed interface

In addition to rocFFT interface, we provide an exact replica of FFTW interface (which hides all GPU programming) to support other languages.

I would say we provide FFTW interfaces to facilitate quick CPU ports. I don't think the primary purpose should be to enable other language wrappers; they should wrap the native C interface.

[bragadeesh]

Thanks for feedback Kent, I am in agreement. I think it would be good to show example wrappers for other languages such as python/fortran, to show the possibility and to provoke interest.

[kknox]

Ya, I think we should provide wrappers for c++ and python. C++ will be more under our direct control. We will need to figure out how to create queues and allocate device memory in python, but i think there will be a way. I think python will be the next easiest, not sure how to make queues or device memory in fortran. It might mean making wrappers for hip or hc.

[pavanky]

Is there any reason both input and output to the execute function are pointers to arrays?https://github.com/RadeonOpenCompute/rocFFT/blob/master/src/include/rocfft.h#L91

Shouldn't the input just be an array and output be pointer to an array ideally ?

[pavanky]

Now that I think about it, neither one of them needs to be a pointer to an array since output is pre-allocated.

Is this done in case the n transforms are not part of the same buffer?

[bragadeesh]

@pavanky the reason is to support planar formats. Both in & out are pointers to array of void-pointers. This 'array' length is either 1 or 2. It is 1 if the data is in complex interleaved format (1 buffer has both real & imaginary) and 2 if the data is in complex planar format (real and imaginary in separate buffers).

[pavanky]

@bragadeesh Yeah that makes sense. thanks for the clarification.