Re: git: 238271f4a66b - main - stand: Add a snarky note about the upstream ZFS situation
Date: Tue, 18 Apr 2023 22:49:57 UTC
On Tue, Apr 18, 2023, 3:34 PM Jessica Clarke <jrtc27@freebsd.org> wrote: > On 18 Apr 2023, at 22:31, Warner Losh <imp@FreeBSD.org> wrote: > > > > The branch main has been updated by imp: > > > > URL: > https://cgit.FreeBSD.org/src/commit/?id=238271f4a66bd06b8b9a232a82f3ee0882e4cbb9 > > > > commit 238271f4a66bd06b8b9a232a82f3ee0882e4cbb9 > > Author: Warner Losh <imp@FreeBSD.org> > > AuthorDate: 2023-04-18 21:29:45 +0000 > > Commit: Warner Losh <imp@FreeBSD.org> > > CommitDate: 2023-04-18 21:31:17 +0000 > > > > stand: Add a snarky note about the upstream ZFS situation > > > > The latest import of openzfs broke the hacks that we used to omit the > > special registers being used on arm64. Add snarky note documenting > this > > situation since it's a mess now since the hack was only partially > > undone, leaving behind a mess. > > > > Sponsored by: Netflix > > --- > > stand/libsa/zfs/Makefile.inc | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/stand/libsa/zfs/Makefile.inc b/stand/libsa/zfs/Makefile.inc > > index f4cecdbc3085..7660f4ab7baf 100644 > > --- a/stand/libsa/zfs/Makefile.inc > > +++ b/stand/libsa/zfs/Makefile.inc > > @@ -19,6 +19,7 @@ ZSTD_SRC+= zstd_common.c > > ZSTD_SRC+= zstd_ddict.c zstd_decompress.c zstd_decompress_block.c > > ZSTD_SRC+= zstd_double_fast.c zstd_fast.c zstd_lazy.c zstd_ldm.c > > > > +# This is completely bogus: We should be able to omit this code > completely. > > .if ${MACHINE_ARCH} == "aarch64" > > ZFS_SRC_AS = b3_aarch64_sse2.S b3_aarch64_sse41.S > > .endif > > @@ -90,10 +91,13 @@ CFLAGS.skein_block.c+= -DSKEIN_LOOP=111 > > > > # To find blake3_impl.c in OpenZFS tree for our somehat ugly > blake3_impl_hack.c > > # that's needed until the necessary tweaks can be upstreamed. > > +# XXX the last import gutted all this since upstream changes broke this > hack. > > CFLAGS.blake3_impl_hack.c+= -I${OZFS}/module/icp/algs/blake3 > -I${OZFS}/module/icp/include > > > > CWARNFLAGS.zfs.c+= ${NO_WDANGLING_POINTER} > > > > +# Needing to remove the -mgeneral-regs-only is a red flag that this is > not quite > > +# right. But it's needed at the moment due to the muddled upstream. > > This one isn’t bogus? The file is deliberately using NEON so needs > access to floating-point registers, which LLVM (mostly) enforces for > the assembler, unlike GNU as. > No. It's bogus because we should not be using it at all. The generic implementation is fast enough for the boot loader and we have a blanket policy against using extra register sets. The change wasn't discussed before hand and what's really needed are upstream changes to be able to omit it entirely. Warner Jess > > > b3_aarch64_sse2.o: b3_aarch64_sse2.S > > ${CC} -c ${CFLAGS:N-mgeneral-regs-only} ${WERROR} ${.IMPSRC} \ > > -o ${.TARGET} > >