Date: Tue, 18 Apr 2023 16:49:57 -0600 From: Warner Losh <imp@bsdimp.com> To: Jessica Clarke <jrtc27@freebsd.org> Cc: Warner Losh <imp@freebsd.org>, src-committers <src-committers@freebsd.org>, "<dev-commits-src-all@freebsd.org>" <dev-commits-src-all@freebsd.org>, "<dev-commits-src-main@freebsd.org>" <dev-commits-src-main@freebsd.org> Subject: Re: git: 238271f4a66b - main - stand: Add a snarky note about the upstream ZFS situation Message-ID: <CANCZdfry1T-xy-xQ19Cch42qmeNNAHFgQf_W61FCbSHWis_aNg@mail.gmail.com> In-Reply-To: <C898F70B-7CA2-451F-A080-ADBFB472561D@freebsd.org> References: <202304182131.33ILVSoG020217@gitrepo.freebsd.org> <C898F70B-7CA2-451F-A080-ADBFB472561D@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--00000000000013f2fd05f9a421fb Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable 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=3D238271f4a66bd06b8b9a232a82f3ee0= 882e4cbb9 > > > > 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 th= e > > 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.in= c > > index f4cecdbc3085..7660f4ab7baf 100644 > > --- a/stand/libsa/zfs/Makefile.inc > > +++ b/stand/libsa/zfs/Makefile.inc > > @@ -19,6 +19,7 @@ ZSTD_SRC+=3D zstd_common.c > > ZSTD_SRC+=3D zstd_ddict.c zstd_decompress.c zstd_decompress_block.c > > ZSTD_SRC+=3D 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} =3D=3D "aarch64" > > ZFS_SRC_AS =3D b3_aarch64_sse2.S b3_aarch64_sse41.S > > .endif > > @@ -90,10 +91,13 @@ CFLAGS.skein_block.c+=3D -DSKEIN_LOOP=3D111 > > > > # 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 thi= s > hack. > > CFLAGS.blake3_impl_hack.c+=3D -I${OZFS}/module/icp/algs/blake3 > -I${OZFS}/module/icp/include > > > > CWARNFLAGS.zfs.c+=3D ${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=E2=80=99t bogus? The file is deliberately using NEON so need= s > 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} > > --00000000000013f2fd05f9a421fb Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable <div dir=3D"auto"><div><br><br><div class=3D"gmail_quote"><div dir=3D"ltr" = class=3D"gmail_attr">On Tue, Apr 18, 2023, 3:34 PM Jessica Clarke <<a hr= ef=3D"mailto:jrtc27@freebsd.org">jrtc27@freebsd.org</a>> wrote:<br></div= ><blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1= px #ccc solid;padding-left:1ex">On 18 Apr 2023, at 22:31, Warner Losh <i= mp@FreeBSD.org> wrote:<br> > <br> > The branch main has been updated by imp:<br> > <br> > URL: <a href=3D"https://cgit.FreeBSD.org/src/commit/?id=3D238271f4a66b= d06b8b9a232a82f3ee0882e4cbb9" rel=3D"noreferrer noreferrer" target=3D"_blan= k">https://cgit.FreeBSD.org/src/commit/?id=3D238271f4a66bd06b8b9a232a82f3ee= 0882e4cbb9</a><br> > <br> > commit 238271f4a66bd06b8b9a232a82f3ee0882e4cbb9<br> > Author:=C2=A0 =C2=A0 =C2=A0Warner Losh <imp@FreeBSD.org><br> > AuthorDate: 2023-04-18 21:29:45 +0000<br> > Commit:=C2=A0 =C2=A0 =C2=A0Warner Losh <imp@FreeBSD.org><br> > CommitDate: 2023-04-18 21:31:17 +0000<br> > <br> >=C2=A0 =C2=A0 stand: Add a snarky note about the upstream ZFS situation= <br> > <br> >=C2=A0 =C2=A0 The latest import of openzfs broke the hacks that we used= to omit the<br> >=C2=A0 =C2=A0 special registers being used on arm64. Add snarky note do= cumenting this<br> >=C2=A0 =C2=A0 situation since it's a mess now since the hack was on= ly partially<br> >=C2=A0 =C2=A0 undone, leaving behind a mess.<br> > <br> >=C2=A0 =C2=A0 Sponsored by:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Net= flix<br> > ---<br> > stand/libsa/zfs/Makefile.inc | 4 ++++<br> > 1 file changed, 4 insertions(+)<br> > <br> > diff --git a/stand/libsa/zfs/Makefile.inc b/stand/libsa/zfs/Makefile.i= nc<br> > index f4cecdbc3085..7660f4ab7baf 100644<br> > --- a/stand/libsa/zfs/Makefile.inc<br> > +++ b/stand/libsa/zfs/Makefile.inc<br> > @@ -19,6 +19,7 @@ ZSTD_SRC+=3D=C2=A0 zstd_common.c<br> > ZSTD_SRC+=3D=C2=A0 =C2=A0 zstd_ddict.c zstd_decompress.c zstd_decompre= ss_block.c<br> > ZSTD_SRC+=3D=C2=A0 =C2=A0 zstd_double_fast.c zstd_fast.c zstd_lazy.c z= std_ldm.c<br> > <br> > +# This is completely bogus: We should be able to omit this code compl= etely.<br> > .if ${MACHINE_ARCH} =3D=3D "aarch64"<br> > ZFS_SRC_AS =3D=C2=A0 b3_aarch64_sse2.S b3_aarch64_sse41.S<br> > .endif<br> > @@ -90,10 +91,13 @@ CFLAGS.skein_block.c+=3D=C2=A0 =C2=A0 -DSKEIN_LOOP= =3D111<br> > <br> > # To find blake3_impl.c in OpenZFS tree for our somehat ugly blake3_im= pl_hack.c<br> > # that's needed until the necessary tweaks can be upstreamed.<br> > +# XXX the last import gutted all this since upstream changes broke th= is hack.<br> > CFLAGS.blake3_impl_hack.c+=3D -I${OZFS}/module/icp/algs/blake3 -I${OZF= S}/module/icp/include<br> > <br> > CWARNFLAGS.zfs.c+=3D ${NO_WDANGLING_POINTER}<br> > <br> > +# Needing to remove the -mgeneral-regs-only is a red flag that this i= s not quite<br> > +# right. But it's needed at the moment due to the muddled upstrea= m.<br> <br> This one isn=E2=80=99t bogus? The file is deliberately using NEON so needs<= br> access to floating-point registers, which LLVM (mostly) enforces for<br> the assembler, unlike GNU as.<br></blockquote></div></div><div dir=3D"auto"= ><br></div><div dir=3D"auto">No. It's bogus because we should not be us= ing it at all. The generic implementation is fast enough for the boot loade= r and we have a blanket policy against using extra register sets. The chang= e wasn't discussed before hand and what's really needed are upstrea= m changes to be able to omit it entirely.</div><div dir=3D"auto"><br></div>= <div dir=3D"auto">Warner</div><div dir=3D"auto"><br></div><div dir=3D"auto"= ><div class=3D"gmail_quote"><blockquote class=3D"gmail_quote" style=3D"marg= in:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Jess<br> <br> > b3_aarch64_sse2.o: b3_aarch64_sse2.S<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0${CC} -c ${CFLAGS:N-mgeneral-regs-only} ${WE= RROR} ${.IMPSRC} \<br> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0-o ${.TARGET}<br> <br> </blockquote></div></div></div> --00000000000013f2fd05f9a421fb--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfry1T-xy-xQ19Cch42qmeNNAHFgQf_W61FCbSHWis_aNg>