q6_K get_rows and dequantize by swetha097 · Pull Request #6 · swetha097/whisper.cpp

swetha097 · 2025-09-10T11:40:06Z

No description provided.

Srihari-mcw

Please address review comments. Thanks

Srihari-mcw · 2025-10-15T05:31:03Z

ggml/src/ggml-cpu/repack.cpp

+
+        switch (src0->type) {
+            case GGML_TYPE_Q6_K:
+                ggml_compute_forward_get_rows_q6_Kx8(params, dst);


Isnt this ideal to be within AVX2 condition similar to Q4_0 ? - https://github.com/ggml-org/whisper.cpp/pull/3223/files

And any reason if (src0->ne[1] % 8 == 0) is skipped? Although add it if only necessary

I think that the block_q4_0x8 is one among the three variants and thats why the condition was necessary but if its better to add here also, maybe try adding it

Srihari-mcw · 2025-10-15T05:43:14Z

ggml/src/ggml-cpu/repack.cpp

 #undef MMID_MATRIX_ROW
    }

+    void forward_get_rows(const ggml_compute_params * params,


Keep all params in same lines, follow same formatting as other functions in file

Srihari-mcw · 2025-10-15T05:43:39Z

ggml/src/ggml-cpu/repack.cpp

+        const ggml_tensor * src0 = dst->src[0];
+        const ggml_tensor * src1 = dst->src[1];
+
+        GGML_TENSOR_BINARY_OP_LOCALS


Add a empty line like https://github.com/ggml-org/whisper.cpp/pull/3223/files

Srihari-mcw · 2025-10-15T05:49:49Z

ggml/src/ggml-cpu/repack.cpp

+        }
+    }
+
+    static void ggml_compute_forward_get_rows_q6_Kx8(


Same comment here, pls follow same formatting as the other functions

Srihari-mcw · 2025-10-15T05:51:20Z

ggml/src/ggml-cpu/repack.cpp

+    /**
+     * Dequantizes a single logical row from the repacked q6_Kx8 data format.
+     *
+     * @param p_repacked_blocks Pointer to the start of the 'block_q6_Kx8' structures for the entire row.


Please make sure that the variable names are uniform across all PRs

Srihari-mcw · 2025-10-15T05:52:41Z

ggml/src/ggml-cpu/repack.cpp

+        float * GGML_RESTRICT y,
+        int64_t k,
+        int row_idx_in_group) {
+        assert(k % QK_K == 0);


Maybe a empty line after the function params would be nice, pls update formatting wherever necessary

Srihari-mcw · 2025-10-15T06:00:57Z

ggml/src/ggml-cpu/repack.cpp

+        }
+    }
+
+    static inline int8_t read_scale_from_repacked(const uint8_t* ptr_repacked_scales, int row_idx_in_group, int scale_idx) {


Just add a line of comment explaining the helper overall? And can we keep these helpers overall at the start of the file only because some other helper AFAIK are also grouped together at the start

Srihari-mcw · 2025-10-15T06:01:21Z

ggml/src/ggml-cpu/repack.cpp

+                    y[l + 96] = d_super_block * sc3 * q4;
+                }
+                y  += 128;
+               ptr_repacked_scales = (uint8_t *)current_block->scales + 64;


Check formatting here

Srihari-mcw · 2025-10-15T06:01:51Z

ggml/src/ggml-cpu/repack.cpp

+
+            const uint8_t * ptr_ql_base = current_block->ql;
+            const uint8_t * ptr_qh_base = current_block->qh;
+            uint8_t * ptr_repacked_scales = (uint8_t *)current_block->scales; // 16*8 scales repacked - 2bytes of each super block stored together


16 * 8, 2 bytes

Srihari-mcw · 2025-10-15T06:03:34Z

ggml/src/ggml-cpu/repack.cpp

+                    const int8_t q3 = (int8_t)((ql_l0  >> 4) | (((qh_l >> 4) & 3) << 4)) - 32;
+                    const int8_t q4 = (int8_t)((ql_l32  >> 4) | (((qh_l >> 6) & 3) << 4)) - 32;
+
+                    y[l +  0] = d_super_block * sc0 * q1;


y[l] should be sufficient imo

Srihari-mcw · 2025-10-15T06:04:45Z

ggml/src/ggml-cpu/repack.cpp

+
+                    const int8_t q1 = (int8_t)((ql_l0 & 0xF) | (((qh_l >> 0) & 3) << 4)) - 32;
+                    const int8_t q2 = (int8_t)((ql_l32 & 0xF) | (((qh_l >> 2) & 3) << 4)) - 32;
+                    const int8_t q3 = (int8_t)((ql_l0  >> 4) | (((qh_l >> 4) & 3) << 4)) - 32;


Check whitespaces between expressions

swetha097 added 2 commits August 18, 2025 23:53

Get rows & Dequantize

de9839e

Remove the print debug statements

d37652e

Srihari-mcw suggested changes Oct 15, 2025

View reviewed changes

Srihari-mcw reviewed Oct 15, 2025

View reviewed changes

Conversation

swetha097 commented Sep 10, 2025

Uh oh!

Srihari-mcw left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants