Skip to content

Fix qd8_f16_qb4w GEMM config getter always returning NULL on Arm#10537

Open
outum1500 wants to merge 1 commit into
google:masterfrom
outum1500:fix-qd8-f16-qb4w-gemm-config-null
Open

Fix qd8_f16_qb4w GEMM config getter always returning NULL on Arm#10537
outum1500 wants to merge 1 commit into
google:masterfrom
outum1500:fix-qd8-f16-qb4w-gemm-config-null

Conversation

@outum1500

Copy link
Copy Markdown

Problem

On Arm, every QD8/F16/QB4W fully-connected op fails at runtime creation with xnn_status_unsupported_hardware, even though the microkernels are built and the hardware supports FP16 (reproduced on Apple M4: hw.optional.arm.FEAT_FP16=1, hardware config reports neon_fp16_arith / neon_dot / neon_i8mm).

Root cause

xnn_init_qd8_f16_qb4w_gemm_config() ends with:

return qd8_f16_qb4w_gemm_config.arch ? &qd8_f16_qb4w_gemm_config : NULL;

but init_qd8_f16_qb4w_gemm_config() never assigns .arch on any branch (it sets only mr / nr / log2_kr / planes / dqgemm). So qd8_f16_qb4w_gemm_config.arch stays 0 and the getter always returns NULL on Arm.

This stays hidden until the convert -> fully_connected packed-LHS path sets XNN_FLAG_INLINE_LHS_PACKING; setup_variant_and_gemm_config() then receives a NULL gemm config and returns xnn_status_unsupported_hardware, aborting runtime creation.

The sibling getters xnn_init_qd8_f16_qc4w_gemm_config(), xnn_init_qd8_f16_qc8w_gemm_config() and xnn_init_qd8_f32_qb4w_gemm_config() return their config unconditionally and likewise do not set .arch. qd8_f16_qb4w is the only one in this family carrying a stray .arch guard.

Fix

Return the config unconditionally, matching the sibling getters.

Test

On Apple M4, a QD8/F16/QB4W linear model that previously aborted at runtime creation with unsupported_hardware now runs to completion. The qd8_f32_qb4w, qd8_f16_qc4w and qd8_f16_qc8w paths are unchanged.

xnn_init_qd8_f16_qb4w_gemm_config() gated its return on
qd8_f16_qb4w_gemm_config.arch, but init_qd8_f16_qb4w_gemm_config() never
assigns .arch on any branch (it sets only mr/nr/log2_kr/planes/dqgemm).
On Arm this makes the getter always return NULL, so every QD8/F16/QB4W
fully-connected op fails runtime creation with
xnn_status_unsupported_hardware once the convert -> fully_connected
packed-LHS path requests it.

Return the config unconditionally, matching the sibling getters
xnn_init_qd8_f16_qc4w_gemm_config() and xnn_init_qd8_f32_qb4w_gemm_config(),
which likewise do not set .arch.

Tested on Apple M4: a QD8/F16/QB4W linear that previously aborted at
runtime creation with unsupported_hardware now runs to completion.
Comment thread src/configs/gemm-config.c
return NULL;
}
XNN_INIT_ONCE(qd8_f16_qb4w_gemm);
return qd8_f16_qb4w_gemm_config.arch ? &qd8_f16_qb4w_gemm_config : NULL;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we set arch, rather than ignoring it?

I think this is only used on x86 to do the qdu8 rewrite, but I'd rather not guess...

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants