SYCL: Remove misleading ggml_sycl_op_flatten function

[qnixsynapse]

Original work of #11515. Tried to submit smaller change this time.

[qnixsynapse]

This function typecasted src0, src1 and dst tensors to float regardless of whatever types they were. I think we don't want this behaviour in the long run.

[acbits]

Just curious, why we don't use auto? You could avoid the casting and if the datatype changes in future, you don't have to rework it again?

Modern C++ code should use auto where ever it is possible.

[qnixsynapse]

Do we need to create a separate pointer variable when we are passing the ggml_tensor itself anyways to the kernel OP function which contains everything needed to perform the OP?

[arthw]

This PR include many code change. But no any bug or feature work.
Because some CI UT cases are skipped manually for 100% pass rate. I'm afraid the updated code can't be verified by CI cases.

So it's high risk to update so much code without 100% UT cover rate.

I suggest refactor them as additional work for bug fix or feature in same source file or class.
So that the code will be covered by test fully.

[qnixsynapse]

This PR include many code change. But no any bug or feature work.
Because some CI UT cases are skipped manually for 100% pass rate. I'm afraid the updated code can't be verified by CI cases.

So it's high risk to update so much code without 100% UT cover rate.

I suggest refactor them as additional work for bug fix or feature in same source file or class.
So that the code will be covered by test fully.

I usually test them locally(by running a model) before opening a PR here. If you want, we can update the CI to run test cases in an available Nvidia ci/cd instance here.

[Rbiessy]

That makes sense to me. We have to be careful when we merge this as it will break #12412 though.

[qnixsynapse]

@Rbiessy I don't think this will break RWKV kernels because they don't depend on ggml_sycl_op_flatten function. I will do a rebase and test.

Please note that this is a prerequisite for supporting F16 types in unary and eltwise operations which I intend to do in future PRs.

[NeoZhangJianyu]

This PR include many code change. But no any bug or feature work.
Because some CI UT cases are skipped manually for 100% pass rate. I'm afraid the updated code can't be verified by CI cases.
So it's high risk to update so much code without 100% UT cover rate.
I suggest refactor them as additional work for bug fix or feature in same source file or class.
So that the code will be covered by test fully.

I usually test them locally(by running a model) before opening a PR here. If you want, we can update the CI to run test cases in an available Nvidia ci/cd instance here.

Test with several models can't cover the OPs.
It's better to run CI.

If your PC has iGPU in intel Core CPU (since 11th or newer Core CPU), you could run the CI on it too.

[Rbiessy]

@Rbiessy I don't think this will break RWKV kernels because they don't depend on ggml_sycl_op_flatten function. I will do a rebase and test.

Please note that this is a prerequisite for supporting F16 types in unary and eltwise operations which I intend to do in future PRs.

I was referring to this line which adds a usage of ggml_sycl_op_flatten. It will be fine if you rebase it now.

[qnixsynapse]

@Rbiessy I don't think this will break RWKV kernels because they don't depend on ggml_sycl_op_flatten function. I will do a rebase and test.
Please note that this is a prerequisite for supporting F16 types in unary and eltwise operations which I intend to do in future PRs.

I was referring to this line which adds a usage of ggml_sycl_op_flatten. It will be fine if you rebase it now.

Ah okay. That can be fixed.

[Alcpz]

Overall looks great to me, thanks for simplifying the code a bit!
Do you have an answer to the try catch questions from @Rbiessy (#12387 (comment))?

I'm happy to give the approval but if you intend to add a few more changes then I will wait until you finish all the stuff.

[qnixsynapse]

Overall looks great to me, thanks for simplifying the code a bit!

Thank you. Will add support for fp16 once this PR gets merged.

Do you have an answer to the try catch questions from @Rbiessy (#12387 (comment))?

I'm happy to give the approval but if you intend to add a few more changes then I will wait until you finish all the stuff.

If you look at my latest changes, I did exactly what @Rbiessy suggested, i.e to add the try - catch only to the compute_forward function and removed it on elementwise ops functions

[Rbiessy]

LGTM, great work!

[Alcpz]

If you look at my latest changes, I did exactly what @Rbiessy suggested, i.e to add the try - catch only to the compute_forward function and removed it on elementwise ops functions

I misunderstood the comment then. LGTM!