get_rows & dequantize function implementation for repacked weights of type q4_0 (q4_0x8)

[swetha097]

• This implements the GGML_OP_GET_ROWS operation specifically for repacked (block interleaved) 4-bit quantized format (q4_0x8) defined within ggml-cpu-aarch64.cpp

• The following gains were observed by the changes made in the PR - The changes allow for increased usage of the GEMM function (ggml_gemm_q4_0_8x8_q8_0) for q4_0 type defined within ggml-cpu-aarch64.cpp

master branch commit - 82f461eaa4e6a1ba29fc0dbdaa415a9934ee8a1d
development (get_rows) branch commit - d6cc466bd48dd27474ecb00c3baba2e8a887f6c4

The PR was tested in AMD Raphael 7600X which supports the following flags :

system_info: n_threads = 4 / 12 | WHISPER : COREML = 0 | OPENVINO = 0 | CPU : SSE3 = 1 | SSSE3 = 1 | AVX = 1 | AVX2 = 1 | F16C = 1 | FMA = 1 | BMI2 = 1 | AVX512 = 1 | AVX512_VBMI = 1 | AVX512_VNNI = 1 | AVX512_BF16 = 1 | LLAMAFILE = 1 | OPENMP = 1 | AARCH64_REPACK = 1 |

Model for performance tests Downloaded from : https://huggingface.co/ggerganov/whisper.cpp/blob/main/ggml-base.en.bin and quantized to q4_0

This patch of the code was also tested with llama.cpp repository & the perplexity of Q4_0 models were ensured to be the same before and after the changes made :

Final estimate: PPL = 5.9625 +/- 0.03348

Model used for perplexity test quantized from - https://huggingface.co/meta-llama/Llama-2-7b

[swetha097]

@danbev Addressed the review comments

[ggerganov]

Let's merge this after the refactoring in ggml-org/llama.cpp#13892 to avoid resolving the conflicts.

[danbev]

@swetha097 Would you be able to take a look at resolving the conflict reported here?

[swetha097]

@danbev Resolved the conflicts.

• Performance with the gcc compiler is stable and matches the previous results.
• To see similar performance gains on clang compiler, we still need to pull in the latest updates from the llama.cpp's ggml library.

[danbev]

@swetha097 Thanks! I think there will be a sync of ggml/llama.cpp shortly, hopefully this won't cause conflict but I'm not sure.

[ggerganov]

@swetha097 Hm, I am a bit confused about why this would make a difference for the performance. The only place that we use ggml_get_rows is here:

whisper.cpp/src/whisper.cpp

Lines 2543 to 2548 in 1e72e4b

	// token encoding + position encoding
	struct ggml_tensor * cur =
	ggml_add(ctx0,
	ggml_get_rows(ctx0, model.d_te, embd),
	ggml_get_rows(ctx0, model.d_pe, position));

How does this change lead to increased usage of the ggml_gemm_q4_0_8x8_q8_0?

[swetha097]

@ggerganov
As noted in whisper-arch.h,

    // Note: ASR_TENSOR_DEC_TOKEN_EMBD_WEIGHT is also used by GGML_OP_MAT_MUL. Need to figure out a way how to handle
    // weight tensors that are used by multiple different operators when extra_buffer_type implementations accelerate
    // more than just GGML_OP_MUL_MAT.
    {ASR_TENSOR_DEC_TOKEN_EMBD_WEIGHT, GGML_OP_GET_ROWS},

this tensor is used by both GGML_OP_MAT_MUL and GGML_OP_GET_ROWS. Because the original get_rows operation could not handle repacked weights, the tensor remained in a standard format, preventing the GGML_OP_MAT_MUL operation from using its most optimized kernel (ggml_gemm_q4_0_8x8_q8_0), instead it was using gemm4xN kernel previously.

[ggerganov]

@swetha097 Thanks, got it.

@eddnjjn Could you help reviewing this change?

[slaren]

Wouldn't this fail if the type of this template is actually block_q4_0x4?

I think we should add support for testing extra buffer types to test-backend-ops before adding more complexity to this code.

[ggerganov]

Yes, I agree. We'll have to add tests before merging more stuff to the repacking.

[swetha097]

@slaren @ggerganov

I have updated the code with templated changes which helps to differentiate between different block_q4_0's (q4_0x8, q4_0x4).

Please do have a look at the changes done and share your feedback on the same.

[swetha097]

@slaren @ggerganov

Are there any other dependencies that should be resolved for the PR to move forward. Please share your thoughts.

[ggerganov]

The main concern here is that we don't have a way to test these changes as noted earlier by slaren. The PR should wait until we extend test-backend-ops to work with extra buffer types so that we can verify modifications.

[swetha097]

@ggerganov

Given that llama.cpp already includes test-backend-ops, would it be acceptable to open this PR in llama.cpp directly?
My changes target the ggml library, which is common to both whisper.cpp and llama.cpp.