Skip to content

Commit

Permalink
lib: Prepare zstd for preboot environment, improve performance
Browse files Browse the repository at this point in the history
These changes are necessary to get the build to work in the preboot
environment, and to get reasonable performance:

- Remove a double definition of the CHECK_F macro when the zstd
  library is amalgamated.

- Switch ZSTD_copy8() to __builtin_memcpy(), because in the preboot
  environment on x86 gcc can't inline `memcpy()` otherwise.

- Limit the gcc hack in ZSTD_wildcopy() to the broken gcc version. See
  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81388.

ZSTD_copy8() and ZSTD_wildcopy() are in the core of the zstd hot loop.
So outlining these calls to memcpy(), and having an extra branch are very
detrimental to performance.

Signed-off-by: Nick Terrell <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Tested-by: Sedat Dilek <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
  • Loading branch information
terrelln authored and Ingo Molnar committed Jul 31, 2020
1 parent 92ed301 commit 6d25a63
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 10 deletions.
9 changes: 1 addition & 8 deletions lib/zstd/fse_decompress.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
****************************************************************/
#include "bitstream.h"
#include "fse.h"
#include "zstd_internal.h"
#include <linux/compiler.h>
#include <linux/kernel.h>
#include <linux/string.h> /* memcpy, memset */
Expand All @@ -60,14 +61,6 @@
enum { FSE_static_assert = 1 / (int)(!!(c)) }; \
} /* use only *after* variable declarations */

/* check and forward error code */
#define CHECK_F(f) \
{ \
size_t const e = f; \
if (FSE_isError(e)) \
return e; \
}

/* **************************************************************
* Templates
****************************************************************/
Expand Down
14 changes: 12 additions & 2 deletions lib/zstd/zstd_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,14 @@ static const U32 OF_defaultNormLog = OF_DEFAULTNORMLOG;
* Shared functions to include for inlining
*********************************************/
ZSTD_STATIC void ZSTD_copy8(void *dst, const void *src) {
memcpy(dst, src, 8);
/*
* zstd relies heavily on gcc being able to analyze and inline this
* memcpy() call, since it is called in a tight loop. Preboot mode
* is compiled in freestanding mode, which stops gcc from analyzing
* memcpy(). Use __builtin_memcpy() to tell gcc to analyze this as a
* regular memcpy().
*/
__builtin_memcpy(dst, src, 8);
}
/*! ZSTD_wildcopy() :
* custom version of memcpy(), can copy up to 7 bytes too many (8 bytes if length==0) */
Expand All @@ -137,13 +144,16 @@ ZSTD_STATIC void ZSTD_wildcopy(void *dst, const void *src, ptrdiff_t length)
const BYTE* ip = (const BYTE*)src;
BYTE* op = (BYTE*)dst;
BYTE* const oend = op + length;
/* Work around https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81388.
#if defined(GCC_VERSION) && GCC_VERSION >= 70000 && GCC_VERSION < 70200
/*
* Work around https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81388.
* Avoid the bad case where the loop only runs once by handling the
* special case separately. This doesn't trigger the bug because it
* doesn't involve pointer/integer overflow.
*/
if (length <= 8)
return ZSTD_copy8(dst, src);
#endif
do {
ZSTD_copy8(op, ip);
op += 8;
Expand Down

0 comments on commit 6d25a63

Please sign in to comment.