Re: git: fb17dfa0c83c - main - libicp: unbreak for armv6 after recent OpenZFS import

On 12 Dec 2023, at 17:01, John Baldwin <jhb@freebsd.org> wrote:
> 
> On 12/8/23 3:10 PM, Dimitry Andric wrote:
>> The branch main has been updated by dim:
>> URL: https://cgit.FreeBSD.org/src/commit/?id=fb17dfa0c83cc213400fe7e1ed7a39253a4fcefa
>> commit fb17dfa0c83cc213400fe7e1ed7a39253a4fcefa
>> Author:     Dimitry Andric <dim@FreeBSD.org>
>> AuthorDate: 2023-12-08 23:09:36 +0000
>> Commit:     Dimitry Andric <dim@FreeBSD.org>
>> CommitDate: 2023-12-08 23:09:50 +0000
>>     libicp: unbreak for armv6 after recent OpenZFS import
>>          The following upstream commit:
>>     727497ccdfcc module/icp/asm-arm/sha2: enable non-SIMD asm kernels on armv5/6
>>          does indeed enable sha2 asm for earlier arm CPUs, but since libicp's
>>     Makefile was not updated, this leads to:
>>            ld: error: undefined reference due to --no-allow-shlib-undefined: zfs_sha256_block_armv7
>>          Fix it by compiling sha256-armv7.S and sha512-armv7.S for armv6 too.
>>          Fixes:          3494f7c019fc
>> ---
>>  cddl/lib/libicp/Makefile | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> diff --git a/cddl/lib/libicp/Makefile b/cddl/lib/libicp/Makefile
>> index 2d9bb3c67cb4..085818f2371a 100644
>> --- a/cddl/lib/libicp/Makefile
>> +++ b/cddl/lib/libicp/Makefile
>> @@ -21,7 +21,7 @@ ASM_SOURCES_AS = \
>>          asm-x86_64/blake3/blake3_sse41.S
>>    CFLAGS+= -D__amd64 -D_SYS_STACK_H -UHAVE_AES
>> -.elif ${MACHINE_ARCH} == "armv7"
>> +.elif ${MACHINE_ARCH} == "armv6" || ${MACHINE_ARCH} == "armv7"
> 
> Since this applies to all 32-bit arm flavors, should this be using
> ${MACHINE_CPUARCH} == "arm" instead?

You may be right; if I read
https://github.com/openzfs/zfs/commit/727497ccdfcc correctly:

> module/icp/asm-arm/sha2: enable non-SIMD asm kernels on armv5/6
> My merged pull request #15557 fixes compilation of sha2 kernels on arm
> v5/6. However, the compiler guards only allows sha256/512_armv7_impl to
> be used when __ARM_ARCH > 6. This patch enables these ASM kernels on all
> arm architectures. Some compiler guards are adjusted accordingly to
> avoid the unnecessary compilation of SIMD (e.g., neon, armv8ce) kernels
> on old architectures.
>
> Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
> Signed-off-by: Shengqi Chen <harry-chen@outlook.com>
> Closes #15623

It's talking about "all arm architectures", but I'm not sure if this
means all arm architectures supported by OpenZFS, or in general. I would
assume the latter.

-Dimitry