Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

why not a union for tblis_scalar? #5

Open
krux02 opened this issue Mar 14, 2017 · 5 comments
Open

why not a union for tblis_scalar? #5

krux02 opened this issue Mar 14, 2017 · 5 comments

Comments

@krux02
Copy link

krux02 commented Mar 14, 2017

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.

@devinamatthews
Copy link
Owner

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
Copy link
Owner

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

@krux02
Copy link
Author

krux02 commented Mar 14, 2017

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.

@devinamatthews
Copy link
Owner

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

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

@krux02
Copy link
Author

krux02 commented Mar 16, 2017

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;
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants