tblis icon indicating copy to clipboard operation
tblis copied to clipboard

why not a union for tblis_scalar?

Open krux02 opened this issue 7 years ago • 5 comments

I am currently working with the C interface of tblis, and one thing that came up to my mind was, why is tblis scalar not implemented with tagged union? To my experience that is the use case that unions are meant to be used.

This is not really an issue, rather a question. So sorry for abusing the issue tracker for such discussions.

krux02 avatar Mar 14 '17 12:03 krux02

Basically laziness: I didn't want to go to the trouble of dealing with complex types spanning C++/C99/C89 and was just looking for something I could easily deal with on the C++ side. However, I have come to notice that setting the scalar value from C is pretty unpleasant (you basically have to use memcpy or pointer casting at this point).

I will consider going with the union approach and/or adding type-specific helper functions to initialize the C structs. Since union members have the same pointer value this should be binary compatible with the current approach.

devinamatthews avatar Mar 14 '17 16:03 devinamatthews

And BTW thanks for the questions, I appreciate your interest in the project.

devinamatthews avatar Mar 14 '17 16:03 devinamatthews

I think a union could be done without too much hassle. I mean you already have predefined len_type and stride_type. Then it shouldn't be too complicated to also add for example single_complex and double_complex. But I only know the frontend, so I don't know the impact that would have on how you deal with it in the backend.

krux02 avatar Mar 14 '17 18:03 krux02

I went ahead and switched to a union and added some convenience functions with c888fcb109d31594d0c51dd374f71de682799ed6. Example:

tblis_scalar s;
tblis_vector v;
s.data.d = 3.0;
tblis_init_vector_s(&v, 100, some_float_ptr, 1);

devinamatthews avatar Mar 14 '17 19:03 devinamatthews

I looked at your code, and tried it out. I do really like the change to the union type. The additional init functions not so much. Compared to what I can already do in C with initializer lists, it is not that much of an improvement, it just adds a bunch of functions to the interface. Maybe it's a personal thing, but I also like to set the type field manually, because that reminds me that there is a tagged union in use, and it isn't even noticeably longer than the init function to write.

tblis_scalar my_scalar = { .type = TYPE_SINGLE, .data = { .s = 10.0f } };
tblis_vector my_vector = { .type = TYPE_SINGLE, .scalar = {.s = 1.0f}, .data = some_float_ptr, .n = 100, .inc = 1 .}

Thanks a lot for the effort. One thing, that I could imagine to be useful though, would be to have the tblis_scalar including the tag as a member of the vector/matrix/tensor types. Then I could set the type tag just by setting the scalar. Then initializers like this for scalars would actually be useful:

// interface
tblis_scalar tblis_scalar_s(float value);
// example usage
void foobar() {
    tblis_vector my_vector = { .scalar = tblis_scalar_s(1.0f), .data = some_float_ptr, .n = 100, .inc = 1 .}
    [...]
}

The last idea I have right in my head, is to have all four tensor types written out, and all in a single union, so that I can have a typed data pointer even in the C interface.

typedef struct {
    type_t type;
    int conj;
    float scalar  __attribute__((__aligned__(8)));
    char unused[12];    
    float* data;
    len_type n;
    stride_type inc;
} tblis_vector_s;

typedef struct {
    type_t type;
    int conj;
    double scalar __attribute__((__aligned__(8)));
    char unused[8];
    double* data;
    len_type n;
    stride_type inc;
} tblis_vector_d;

typedef struct {
    type_t type;
    int conj;
    scomplex scalar __attribute__((__aligned__(8)));
    char unused[8];
    scomplex* data;
    len_type n;
    stride_type inc;
} tblis_vector_sc;

typedef struct {
    type_t type;
    int conj;
    dcomplex scalar __attribute__((__aligned__(8)));
    dcomplex* data;
    len_type n;
    stride_type inc;
} tblis_vector_dc;

typedef union {
    type_t type;
    tblis_vector_s s;
    tblis_vector_d d;
    tblis_vector_sc sc;
    tblis_vector_dc dc;
};

krux02 avatar Mar 16 '17 11:03 krux02