[Packing Refactor] Move all Blockwise Packing to pack_weights_and_bias

[mcr229]

Packing in the fully-connected operator level APIs has become quite messy because of the separate packing signatures for pack_gemm_goi and pack_gemm_goi_bl. To clean this up, we move the packing(and scale + bias initialization) into a new pack_weights_and_bias function.

One change from this is that we have to add the block_size argument to the signature of xnn_pack_weights_and_biases_fn. This is largely unused for most packing functions. Another change to make this cleaner requires including the microparams-init.h header in the reference/packing.cc file.

See:
src/operators/fully-connected-nc.c

for the cleaning up of the packing.

[fbarchard]

can these use gemm-config to set the packw microkernels?

[mcr229]

do you mean using something like:

gemm_config->packw_gemm_goi(

I was contemplating this but i was looking at config-types.h:

XNNPACK/src/xnnpack/config-types.h

Lines 189 to 197 in c535478

	// TODO(b/346765736): Replace all uses of packing functions with this.
	xnn_pack_weights_and_biases_fn pack_weights_and_biases;
	xnn_packed_stride_weights_and_biases_fn packed_stride_weights_and_biases;
	// Deprecated. Use pack_weights_and_biases instead.
	xnn_packw_gemm_gio_ukernel_fn pack_gemm_gio;
	// Deprecated. Use pack_weights_and_biases instead.
	xnn_packw_gemm_goi_ukernel_fn pack_gemm_goi;
	// TODO(b/346765736): Use pack_weights_and_biases instead.
	xnn_packw_gemm_goi_bl_ukernel_fn pack_gemm_goi_bl;

and there seem to be active tasks to instead move all of these into pack_weights_and_biases instead. So I went ahead and helped towards this by removing packw_gemm_goi_bl from the config types. Not sure what the intended direction for this is though. Perhaps @alankelly can chime in.

[alankelly]

This is the correct way of calling packing functions. Thanks for the clean up, this is much better

[mcr229]

i was discussing with @fbarchard and I think we might want to keep the

xnn_packw_gemm_goi_bl_ukernel_fn pack_gemm_goi_bl; 

in the config-types so that we can specify the non reference packing ukernels in gemm-config, and the pack_weights_and_biases will pull the actual packing ukernel from the gemm-config. So in a sense pack_weights_and_biases will be the wrapper around packw_gemm_goi_ukernel, and will also fill in scales, biases at the same time.

Was wondering if you had any thoughts on that

[fbarchard]

I'm not sure where those deprecated comments came from but my main concern is that the pack_weights_and_biases is hard coded to the reference packing functions, preventing them from being removed in favor of scalar microkernels.

If we can make them call the configured microkernels, performance would be improved.

Secondary concern is that GIO packing is a transpose compared to GOI. Unless there is a usecase, the GIO packing functions will likely be less optimized scalar kernels.

I've noticed packing signature mismatches. I think this occurred when params were changed in packing.cc but the packw microkernels were not updated to the same signature.
They should match. Whatever signature packing.cc has, the packw should be idential, and the function pointers, tests and benchmarks should be able to use the microkernels.

[mcr229]

hi @alankelly I added back

xnn_packw_gemm_goi_bl_ukernel_fn pack_gemm_goi_bl; 

As I think it might be useful in the future, would appreciate another look Thanks!