clang14.0.7 from Android NDK crashes when compiling bf16-gemm-1x4c8-minmax-neonbf16-bfdot.c about xnnpack

[leleliu008]

step1:

sed -i 's|-march=armv6 -mfpu=vfp|-march=armv7-a -mfpu=neon|' CMakeLists.txt

step2:
add cmake option: -DXNNPACK_ENABLE_ARM_BF16=OFF

successfully built now, but don't know if the right way.

[ngzhian]

Manage to get the repro steps down to:

android-ndk-r25c/toolchains/llvm/prebuilt/linux-x86_64/bin/clang --target=armv7a-linux-androideabi21 -Iinclude -Isrc -O1 -march=armv8.2-a+bf16 -mfpu=neon-fp-armv8 src/bf16-gemm/gen/bf16-gemm-6x8c2-minmax-neonbf16-bfdot-lane-ld128.c

Note that -O1, -O2, -O3 fails, but -O0 is fine.

taking a closer look

[ngzhian]

add cmake option: -DXNNPACK_ENABLE_ARM_BF16=OFF is the easiest way, it doesn't include the problematic c file.
@georgthegreat if you are getting blocked, that's probably the easiest way.

[ngzhian]

https://godbolt.org/z/656PqT6o1
I think it is a clang bug, 16.0.0 fixes it.

[ngzhian]

I found that I can workaround the bug with changing:

const uint32x4_t va0c01 = vdupq_lane_u32(vreinterpret_u32_bf16(vget_low_bf16(va0)), 0);

to

const uint32x4_t va0c01 = vdupq_lane_u32(vget_low_u32(vreinterpretq_u32_bf16(va0)), 0);

just the ordering of interpret and get_low.

[ngzhian]

vout0x0123 = vreinterpret_bf16_u16(vext_u16(vreinterpret_u16_bf16(vout0x0123), vreinterpret_u16_bf16(vout0x0123), 2)); is still a problem though

I think i can change it to vout0x0123 = vdup_lane_bf16(vout0x0123, 2);

[ngzhian]

Hm, looks like we don't test building of these microkernels: https://github.com/google/XNNPACK/blob/master/scripts/build-android-armv7.sh#L59

We might also need to update the ndk version we are testing with: https://github.com/google/XNNPACK/blob/master/.github/workflows/build.yml#LL172C28-L172C28

[georgthegreat]

Hm, looks like we don't test building of these microkernels:

There is even a comment about this specific issue:

# BF16 instructions cause ICE in Android NDK compiler

[ngzhian]

I think the easiest fix is for you to add this compile flag when building XNNPACK -DXNNPACK_ENABLE_ARM_BF16=OFF, until the ndk ships a newer version of clang.

[georgthegreat]

Yep, this is what I did for now.

[georgthegreat]

NDK maintainers claim that canary builds with clang16 can compile this code.
I think there is nothing to be done from xnnpack side, so this may be closed if there is no workaround / nobody wants to search for it.

[RobertFlatt]

As I read the posts above there are two possible workarounds, either could be implemented in XNNPACK.

Just waiting for some future NDK where some future Clang will be fixed, is I think an insufficient response.

[Maratyszcza]

The recommended work-around is to disable ARM BF16 extensions using either -DXNNPACK_ENABLE_ARM_BF16=OFF option (for CMake) or --define xnn_enable_arm_bf16=false option (for Bazel).

[RobertFlatt]

Using tensorflow 2.13.0-rc1 and -DXNNPACK_ENABLE_ARM_BF16=OFF

cmake  -DCMAKE_TOOLCHAIN_FILE=../android-ndk-r25c/build/cmake/android.toolchain.cmake -DANDROID_ABI=armeabi-v7a -DTFLITE_ENABLE_XNNPACK=ON -DXNNPACK_ENABLE_ARM_BF16=OFF ../tensorflow_src/tensorflow/lite  
cmake --build . -j

Does not workaround the issue.

And we should not expect it to because this is defined as the default behavior https://github.com/google/XNNPACK/blob/master/scripts/build-android-armv7.sh#L59 and we know the default fails to build.
Not clear why it was suggested as a workaround.

I've been trying to get some attention paid to this for 3.5 months #4348
Please implement a fix.
It seems reasonable to suggest that: the relationship between the code and the build tools is everywhere and always the responsibility of the programmer responsible for the code.