add tests for matrix size be runtime dimension

[YixingZhang007]

No description provided.

[dkhaldi]

You should be able to reuse all the of the code in joint_matrix_bf16_fill_k_cache_impl.hpp joint_matmul function.
Only the signature would change.
You may want to outline the common code and use it in both hpp files.

Also, for the cpp file, you can call it joint_matrix_bf16_fill_k_cache_runtime_dim.cpp for shorter name.

[dkhaldi]

Added comments

[dkhaldi]

What about the runtime arguments test? I think we need that one as well.

[YixingZhang007]

What about the runtime arguments test? I think we need that one as well.

Thanks for all the suggestions for this PR 👍

I am still working on this one. I’m trying to do it in a similar way as we have for joint_matrix_bf16_cache_arg_dim.cpp, where we reuse the code in joint_matrix_bf16_cache_impl.hpp. However, I found that there will be a lot of modifications needed in joint_matrix_bf16_cache_impl.hpp because we need to change the interface for the test() function. So, I’m not sure if I should create a separate *_impl.hpp for the joint_matrix_bf16_cache_runtime_dim test.

[dkhaldi]

I am still working on this one. I’m trying to do it in a similar way as we have for joint_matrix_bf16_cache_arg_dim.cpp, where we reuse the code in joint_matrix_bf16_cache_impl.hpp. However, I found that there will be a lot of modifications needed in joint_matrix_bf16_cache_impl.hpp because we need to change the interface for the test() function. So, I’m not sure if I should create a separate *_impl.hpp for the joint_matrix_bf16_cache_runtime_dim test.

You may create a new joint_matrix_bf16_cache_runtime_dim_impl.hpp file that only contains the new test() function but uses same joint_matmul function from joint_matrix_bf16_cache_impl.hpp.

[YixingZhang007]

I am still working on this one. I’m trying to do it in a similar way as we have for joint_matrix_bf16_cache_arg_dim.cpp, where we reuse the code in joint_matrix_bf16_cache_impl.hpp. However, I found that there will be a lot of modifications needed in joint_matrix_bf16_cache_impl.hpp because we need to change the interface for the test() function. So, I’m not sure if I should create a separate *_impl.hpp for the joint_matrix_bf16_cache_runtime_dim test.

You may create a new joint_matrix_bf16_cache_runtime_dim_impl.hpp file that only contains the new test() function but uses same joint_matmul function from joint_matrix_bf16_cache_impl.hpp.

Thanks for the suggestion! I have created a new joint_matrix_bf16_cache_runtime_dim_impl.hpp.

[YuriPlyakhin]

I suggest not to introduce this file with mostly duplicated code.
Instead, can we add one more macro in the sycl/test-e2e/Matrix/joint_matrix_bf16_fill_k_cache_impl.hpp, like

#if runtime_matrix_size
    void test(size_t matrix_size) {
#else
    void test() {
        constexpr size_t matrix_size = MATRIX_SIZE;
#endif

and propagate the change up to main and down to kernel call?

[YixingZhang007]

Thanks for the suggestion! I have modified main to include the following,

int main(
#ifdef RUNTIME_DIM
  int argc, char *argv[]
#endif //RUNTIME_DIM
  ) {

size_t matrix_size = MATRIX_SIZE;
#ifdef RUNTIME_DIM
  // Check for command line argument
  if (argc == 2) {
    matrix_size = std::stoul(argv[1]);
  } else {
    std::cerr << "Usage: ./program matrix_size\n";
    return 1; // Error if no argument
  }
#endif //RUNTIME_DIM

and modified test as below,

void test(size_t matrix_size) 

This could help eliminate duplicate code for calling the test function in main. Please let me know if you’d prefer to use another way instead (having two interface for test function and having different calls to test in main).