Re: Unbreak zstd on strict alignment architectures

On Fri 03/03/2023 12:35, Theo Buehler wrote:
> On Fri, Mar 03, 2023 at 09:42:42AM +0000, Stuart Henderson wrote:
> > On 2023/03/03 06:49, Bjorn Ketelaars wrote:
> > > On Fri 03/03/2023 01:55, Theo Buehler wrote:
> > > > zstd is broken on sparc64 due to unaligned accesses resulting in bus
> > > > errors. Since __GNUC__ is defined and MEM_FORCE_MEMORY_ACCESS isn't
> > > > defined, it defaults to 1, and we run into these:
> > > >
> > > > https://github.com/facebook/zstd/blob/1be95291a89160be121c987c2e385331a65a4a0e/lib/common/mem.h#L192-L199
> > > >
> > > > One simple fix is to define MEM_FORCE_MEMORY_ACCESS to 0 via CPPFLAGS.
> > > > This forces all platforms to use memcpy, which may or may not result in
> > > > some slowdown. We can consider more elaborate platform-specific fixes if
> > > > that is desired, but then I need instructions.
> > > >
> > > > With this diff, the cmake test for zstd doesn't abort with a bus error,
> > > > which allows me to build qt6/qtbase on sparc64, and in turn we should
> > > > get a sizable chunk of the tree back to building.
> >
> > So it's from this commit
> >
> > https://github.com/facebook/zstd/commit/a78c91ae59b9487fc32224b67c4a854dc3720367
>
> This diff isn't a clean revert, of course.
>
> > > I did some light benchmarking using your patch and first impression,
> > > i.e. looking at the numbers, is that there is no slow dowm. I did *not*
> > > perform statistical analysis to underpin this observation.
> > >
> > > That said I have a slight preference for making this change platform
> > > specific, and tested https://pbot.rmdir.de/9-6H2Qy6ah89TpUnMYCzeg.
> >
> > If we're taking that approach, can we have some test (e.g. in
> > post-build) so that zstd build fails on an arch that requires this
> > so we can keep the list up to date more easily?
>
> Something like this would be enough:
>
> post-build:
> ${WRKRSC}/zstd -o /dev/null /etc/rc
>
> Honestly, I think this is going too far. Also, we might not get this
> into a bulk on some potentially affected hardware (powerpc*?) before
> release packages are built. As mentioned, this breaks the cmake zstd
> detection, and I expect a good number of ports to be affected. I think
> release is getting close too fast to play such games.
>
> Another possibility is to do
>
> --- lib/common/mem.h.patch.orig Fri Mar 3 07:38:13 2023
> +++ lib/common/mem.h Fri Mar 3 07:38:33 2023
> @@ -141,7 +141,8 @@ MEM_STATIC size_t MEM_swapST(size_t in);
> * Default : method 1 if supported, else method 0
> */
> #ifndef MEM_FORCE_MEMORY_ACCESS /* can be defined externally, on command line for example */
> -# ifdef __GNUC__
> +# include <endian.h>
> +# if defined(__GNUC__) && !defined(__STRICT_ALIGNMENT)
> # define MEM_FORCE_MEMORY_ACCESS 1
> # endif
>