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

Compilation failure with the custom data type #1656

Open
zjing14 opened this issue Nov 11, 2024 · 7 comments
Open

Compilation failure with the custom data type #1656

zjing14 opened this issue Nov 11, 2024 · 7 comments
Assignees

Comments

@zjing14
Copy link
Contributor

zjing14 commented Nov 11, 2024

Got the compilation error with a custom data type

/home/jizhan/composable_kernel_pk_i4/include/ck/utility/dynamic_buffer.hpp:344:56: error: object of type 'ck::non_native_vector_base<ck::pk_i4_t, 16>' cannot be assigned because its copy assignment operator is implicitly deleted
  344 |                 *c_style_pointer_cast<X*>(&p_data_[i]) = x;
      |                                                        ^

Below is a simple reproduction.

    struct custom_f8_t
    {
        using type = _BitInt(8);
        type data;
        custom_f8_t() : data{type{}} {}
        custom_f8_t(type init) : data{init} {}
    };

    // test size
    const int size                   = 4;
    std::vector<_BitInt(8)> test_vec = {type_convert<_BitInt(8)>(0.3f),
                                        type_convert<_BitInt(8)>(-0.6f),
                                        type_convert<_BitInt(8)>(0.8f),
                                        type_convert<_BitInt(8)>(-0.2f)};
    // reference vector
    vector_type<custom_f8_t, size> right_vec;
    // check default CTOR
    ck::static_for<0, size, 1>{}(
        [&](auto i) { ASSERT_EQ(right_vec.template AsType<custom_f8_t>()(Number<i>{}).data, 0); });
    // assign test values to the vector
    ck::static_for<0, size, 1>{}([&](auto i) {
        right_vec.template AsType<custom_f8_t>()(Number<i>{}) = custom_f8_t{test_vec.at(i)};
    });

    //Compilation Error: cannot be assigned because its copy assignment operator is
    //implicitly deleted
    vector_type<custom_f8_t, size> left_vec;
    left_vec = right_vec;
@zjing14
Copy link
Contributor Author

zjing14 commented Nov 11, 2024

@geyyer @carlushuang

@geyyer
Copy link
Contributor

geyyer commented Nov 12, 2024

Hi @zjing14, thanks for reporting this issue. The repro code you are using is a part of our test and is run daily. Could you share more details about your build setup so we can compare it to our CI?

@geyyer geyyer self-assigned this Nov 12, 2024
@zjing14
Copy link
Contributor Author

zjing14 commented Nov 12, 2024

@geyyer It is not an issue in the existing code. I am trying to add pk_int4 as a custom data type rather uint8_t, which is already used for bfp8.

You can reproduce the issue with the modified test code as below.

struct custom_f8_t
    {
        using type = _BitInt(8);
        type data;
        custom_f8_t() : data{type{}} {}
        custom_f8_t(type init) : data{init} {}
    };

    // test size
    const int size                   = 4;
    std::vector<_BitInt(8)> test_vec = {type_convert<_BitInt(8)>(0.3f),
                                        type_convert<_BitInt(8)>(-0.6f),
                                        type_convert<_BitInt(8)>(0.8f),
                                        type_convert<_BitInt(8)>(-0.2f)};
    // reference vector
    vector_type<custom_f8_t, size> right_vec;
    // check default CTOR
    ck::static_for<0, size, 1>{}(
        [&](auto i) { ASSERT_EQ(right_vec.template AsType<custom_f8_t>()(Number<i>{}).data, 0); });
    // assign test values to the vector
    ck::static_for<0, size, 1>{}([&](auto i) {
        right_vec.template AsType<custom_f8_t>()(Number<i>{}) = custom_f8_t{test_vec.at(i)};
    });

    //Compilation Error: cannot be assigned because its copy assignment operator is
    //implicitly deleted
    vector_type<custom_f8_t, size> left_vec;
    left_vec = right_vec;

@geyyer
Copy link
Contributor

geyyer commented Nov 13, 2024

@zjing14, I see your point. Would a copy constructor like vector_type<custom_f8_t, size> left_vec{right_vec}; work for you? Or would you like a copy assignment operator to be added?

@zjing14
Copy link
Contributor Author

zjing14 commented Nov 14, 2024

@geyyer a copy assignment is needed otherwise we need to change a lot of places in copy and dynamc_buffer.

@geyyer
Copy link
Contributor

geyyer commented Nov 14, 2024

@zjing14, sure, we'll add it!

@mtgu0705
Copy link

@geyyer, Could you please assist in expediting the progress of this issue?

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

No branches or pull requests

4 participants