ggml-backend : code style suggestions

[ggerganov]

@slaren I'm still reviewing - will continue after lunch

Here are some suggestions about normalizing the style to be more inline with the core ggml implementation.
Let me know if you agree.

Main changes are:

• more systematic prefixes (i.e. ggml_buffer_context_t -> ggml_backend_buffer_context_t
• avoid typedef for struct * - I prefer to have the type spelled out

[slaren]

I don't have any strong opinions about the naming conventions, but one of the reasons why I added the typedef'ed _t pointer types is because I don't want the users to think about these types as structs that they can modify themselves. When we do that, the structs become part of the API and any changes risk breaking user code. So I would very much prefer to hide all of these details, and only expose a few opaque handle types and the functions to use them, and hide as much of the internals as possible. However, that would also require adding more files to separate between the internal backend API for backend implementations, and the final user API.

[ggerganov]

The best way to prevent from users fiddling with these structs is to have them declared in ggml-backend.c and use only forward declarations in the public API. I've pushed a suggestion for this with e1351e9

[slaren]

This breaks ggml-cuda, because it needs the definitions of ggml_backend and ggml_backend_buffer to access the context and initialize the backend. This could be somewhat fixed by adding a ggml_backend_init function and an accessor for the contexts, but then these are details that shouldn't be exposed to the user either.

I am not sure that this can be completely fixed without splitting ggml-backend.h into the user API and the backend API. The interface structs shouldn't be exposed either, even if they should be useless without access to the buffer/backend structs.

[slaren]

The size is used by ggml-alloc, so if this is moved to the context, we would also need to add a function in the interface to get the size. But I am not sure that there is any reason to encapsulate this.

[ggerganov]

Ok, I see your point. We should implement the backend API separation (user / backend), but we can do this for v2. For now I've moved the structs back to ggml-backend.h

one of the reasons why I added the typedef'ed _t pointer types is because I don't want the users to think about these types as structs that they can modify themselves.

Modify in the sense of mutating the data of the structs from user code or modifying the actual structs such as adding / removing struct members? I don't think that the typedefs help with future proofing the code. The data is still exposed and nothing prevents the users from accessing it.

[slaren]

Modify in the sense of mutating the data of the structs from user code or modifying the actual structs such as adding / removing struct members?

The data from the user code. I want to avoid a situation like ggml_cgraph, that has become so widely used in user code, that changing any details about it has become very hard.

The way I think about the _t typedefs is similar to the other typedefs to void * like the context_t types, from the user perspective this should be just an opaque type that they know nothing about. That said, in practice it is the same if we just use pointers to structs with a forward declaration but without the full definition of the struct, so I think this mostly a matter of style and I don't really have a strong opinion either way.

[slaren]

A bit of the rationale about the _t types comes from the linux kernel style guide: https://www.kernel.org/doc/html/v4.10/process/coding-style.html#typedefs

Lots of people think that typedefs ``help readability``. Not so. They are
useful only for:

 (a) totally opaque objects (where the typedef is actively used to **hide**
     what the object is).

     Example: ``pte_t`` etc. opaque objects that you can only access using
     the proper accessor functions.

It's true that currently it doesn't hide anything because there still isn't a separation between user and backend API, it is more a declaration of intent at this point.

[ggerganov]

Ok, let's give it a try. The change to remove typedefs is always trivial, so we can do it in the future if we think we don't benefit from the extra type indirection