Skip to content

Commit

Permalink
[AArch64] Use crc32 instructions when available
Browse files Browse the repository at this point in the history
The time goes from 0.838s down to 0.029s (a 28x speedup) on a Graviton A1
instance and the following benchmark:

function simple_crc32() {
  $a = "foo";
  for ($i = 0; $i < 10000; $i++) {
      crc32($a);
      $a .= "o".$i;
  }
}
  • Loading branch information
sebpop authored and nikic committed Jul 11, 2019
1 parent ba8c489 commit 2a535a9
Showing 1 changed file with 50 additions and 0 deletions.
50 changes: 50 additions & 0 deletions ext/standard/crc32.c
Expand Up @@ -20,6 +20,32 @@
#include "basic_functions.h"
#include "crc32.h"

#if defined(__aarch64__)
# pragma GCC target ("+nothing+crc")
# include <arm_acle.h>
# if defined(__linux__)
# include <sys/auxv.h>
# include <asm/hwcap.h>
# endif

static inline int has_crc32_insn() {
/* Only go through the runtime detection once. */
static int res = -1;
if (res != -1)
return res;
# if defined(HWCAP_CRC32)
res = getauxval(AT_HWCAP) & HWCAP_CRC32;
return res;
# elif defined(HWCAP2_CRC32)
res = getauxval(AT_HWCAP2) & HWCAP2_CRC32;
return res;
# else
res = 0;
return res;
# endif
}
#endif

/* {{{ proto string crc32(string str)
Calculate the crc32 polynomial of a string */
PHP_NAMED_FUNCTION(php_if_crc32)
Expand All @@ -35,6 +61,30 @@ PHP_NAMED_FUNCTION(php_if_crc32)

crc = crcinit^0xFFFFFFFF;

#if defined(__aarch64__)
if (has_crc32_insn()) {
while(nr >= sizeof(uint64_t)) {
crc = __crc32d(crc, *(uint64_t *)p);
p += sizeof(uint64_t);
nr -= sizeof(uint64_t);
}
if (nr >= sizeof(int32_t)) {
crc = __crc32w(crc, *(uint32_t *)p);
p += sizeof(uint32_t);
nr -= sizeof(uint32_t);
}
if (nr >= sizeof(int16_t)) {
crc = __crc32h(crc, *(uint16_t *)p);
p += sizeof(uint16_t);
nr -= sizeof(uint16_t);
}
if (nr) {
crc = __crc32b(crc, *p);
p += sizeof(uint8_t);
nr -= sizeof(uint8_t);
}
}
#endif
for (; nr--; ++p) {
crc = ((crc >> 8) & 0x00FFFFFF) ^ crc32tab[(crc ^ (*p)) & 0xFF ];
}
Expand Down

9 comments on commit 2a535a9

@remicollet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this commit breaks aarch64 build

In file included from /builddir/build/BUILD/php-7.4.0RC3/main/php.h:37,
                 from /builddir/build/BUILD/php-7.4.0RC3/ext/standard/crc32.c:19:
/builddir/build/BUILD/php-7.4.0RC3/ext/standard/crc32.c: In function 'php_if_crc32':
/builddir/build/BUILD/php-7.4.0RC3/Zend/zend_API.h:1621:31: error: inlining failed in call to always_inline 'zend_parse_arg_string': target specific option mismatch
 1621 | static zend_always_inline int zend_parse_arg_string(zval *arg, char **dest, size_t *dest_len, int check_null)
      |                               ^~~~~~~~~~~~~~~~~~~~~
In file included from /builddir/build/BUILD/php-7.4.0RC3/Zend/zend_types.h:25,
                 from /builddir/build/BUILD/php-7.4.0RC3/Zend/zend.h:27,
                 from /builddir/build/BUILD/php-7.4.0RC3/main/php.h:33,
                 from /builddir/build/BUILD/php-7.4.0RC3/ext/standard/crc32.c:19:
/builddir/build/BUILD/php-7.4.0RC3/Zend/zend_API.h:1478:19: note: called from here
 1478 |   if (UNEXPECTED(!zend_parse_arg_string(_arg, &dest, &dest_len, check_null))) { \
      |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/builddir/build/BUILD/php-7.4.0RC3/Zend/zend_portability.h:323:52: note: in definition of macro 'UNEXPECTED'
  323 | # define UNEXPECTED(condition) __builtin_expect(!!(condition), 0)
      |                                                    ^~~~~~~~~
/builddir/build/BUILD/php-7.4.0RC3/Zend/zend_API.h:1485:2: note: in expansion of macro 'Z_PARAM_STRING_EX2'
 1485 |  Z_PARAM_STRING_EX2(dest, dest_len, check_null, separate, separate)
      |  ^~~~~~~~~~~~~~~~~~
/builddir/build/BUILD/php-7.4.0RC3/Zend/zend_API.h:1488:2: note: in expansion of macro 'Z_PARAM_STRING_EX'
 1488 |  Z_PARAM_STRING_EX(dest, dest_len, 0, 0)
      |  ^~~~~~~~~~~~~~~~~
/builddir/build/BUILD/php-7.4.0RC3/ext/standard/crc32.c:59:3: note: in expansion of macro 'Z_PARAM_STRING'
   59 |   Z_PARAM_STRING(p, nr)
      |   ^~~~~~~~~~~~~~
In file included from /builddir/build/BUILD/php-7.4.0RC3/main/php.h:37,
                 from /builddir/build/BUILD/php-7.4.0RC3/ext/standard/crc32.c:19:
/builddir/build/BUILD/php-7.4.0RC3/Zend/zend_API.h:1621:31: error: inlining failed in call to always_inline 'zend_parse_arg_string': target specific option mismatch
 1621 | static zend_always_inline int zend_parse_arg_string(zval *arg, char **dest, size_t *dest_len, int check_null)
      |                               ^~~~~~~~~~~~~~~~~~~~~
In file included from /builddir/build/BUILD/php-7.4.0RC3/Zend/zend_types.h:25,
                 from /builddir/build/BUILD/php-7.4.0RC3/Zend/zend.h:27,
                 from /builddir/build/BUILD/php-7.4.0RC3/main/php.h:33,
                 from /builddir/build/BUILD/php-7.4.0RC3/ext/standard/crc32.c:19:
/builddir/build/BUILD/php-7.4.0RC3/Zend/zend_API.h:1478:19: note: called from here
 1478 |   if (UNEXPECTED(!zend_parse_arg_string(_arg, &dest, &dest_len, check_null))) { \
      |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/builddir/build/BUILD/php-7.4.0RC3/Zend/zend_portability.h:323:52: note: in definition of macro 'UNEXPECTED'
  323 | # define UNEXPECTED(condition) __builtin_expect(!!(condition), 0)
      |                                                    ^~~~~~~~~
/builddir/build/BUILD/php-7.4.0RC3/Zend/zend_API.h:1485:2: note: in expansion of macro 'Z_PARAM_STRING_EX2'
 1485 |  Z_PARAM_STRING_EX2(dest, dest_len, check_null, separate, separate)
      |  ^~~~~~~~~~~~~~~~~~
/builddir/build/BUILD/php-7.4.0RC3/Zend/zend_API.h:1488:2: note: in expansion of macro 'Z_PARAM_STRING_EX'
 1488 |  Z_PARAM_STRING_EX(dest, dest_len, 0, 0)
      |  ^~~~~~~~~~~~~~~~~
/builddir/build/BUILD/php-7.4.0RC3/ext/standard/crc32.c:59:3: note: in expansion of macro 'Z_PARAM_STRING'
   59 |   Z_PARAM_STRING(p, nr)
      |   ^~~~~~~~~~~~~~
make: *** [Makefile:1856: ext/standard/crc32.lo] Error 1

@remicollet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I propose to revert this minor optimization

Tracked on https://bugs.php.net/bug.php?id=78622

@nikic
Copy link
Member

@nikic nikic commented on 2a535a9 Oct 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@remicollet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@remicollet
Copy link
Contributor

@remicollet remicollet commented on 2a535a9 Oct 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikic

/bin/sh /builddir/build/BUILD/php-7.4.0RC3/build-cgi/libtool --silent --preserve-dup-deps --mode=compile cc -DZEND_ENABLE_STATIC_TSRMLS_CACHE=1 -Iext/standard/ -I/builddir/build/BUILD/php-7.4.0RC3/ext/standard/ -DPHP_ATOM_INC -I/builddir/build/BUILD/php-7.4.0RC3/build-cgi/include -I/builddir/build/BUILD/php-7.4.0RC3/build-cgi/main -I/builddir/build/BUILD/php-7.4.0RC3 -I/builddir/build/BUILD/php-7.4.0RC3/build-cgi/ext/date/lib -I/builddir/build/BUILD/php-7.4.0RC3/ext/date/lib -I/usr/include/libxml2 -I/usr/include/enchant -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/imap -I/builddir/build/BUILD/php-7.4.0RC3/ext/mbstring/libmbfl -I/builddir/build/BUILD/php-7.4.0RC3/build-cgi/ext/mbstring/libmbfl -I/builddir/build/BUILD/php-7.4.0RC3/ext/mbstring/libmbfl/mbfl -I/builddir/build/BUILD/php-7.4.0RC3/build-cgi/ext/mbstring/libmbfl/mbfl -I/usr/include/firebird -I/usr/include/pspell -I/usr/include/editline -I/builddir/build/BUILD/php-7.4.0RC3/build-cgi/TSRM -I/builddir/build/BUILD/php-7.4.0RC3/build-cgi/Zend -I/builddir/build/BUILD/php-7.4.0RC3/main -I/builddir/build/BUILD/php-7.4.0RC3/Zend -I/builddir/build/BUILD/php-7.4.0RC3/TSRM -I/builddir/build/BUILD/php-7.4.0RC3/build-cgi/    -I/usr/include -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -fasynchronous-unwind-tables -fstack-clash-protection -fno-strict-aliasing -Wno-pointer-sign -fvisibility=hidden -Wall -Wno-strict-aliasing -DZEND_SIGNALS   -c /builddir/build/BUILD/php-7.4.0RC3/ext/standard/crc32.c -o ext/standard/crc32.lo 
/builddir/build/BUILD/php-7.4.0RC3/ext/standard/crc32.c:48:1: error: pragma or attribute 'target("crc")' is not valid
   48 | uint32_t crc32_aarch64(uint32_t crc, char *p, size_t nr) {
      | ^~~~~~~~
make: *** [Makefile:1856: ext/standard/crc32.lo] Error 1
make: *** Waiting for unfinished jobs....

@nikic
Copy link
Member

@nikic nikic commented on 2a535a9 Oct 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@remicollet Here's another version: https://gist.github.com/nikic/51301da72853b3c3ad7659eba02232d1

If this doesn't work, let's revert this.

@remicollet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikic see https://koji.fedoraproject.org/koji/taskinfo?taskID=38003042

Build still running, seems OK, only waiting for test suite results

@remicollet
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nikic build done, everything is OK

Please apply your fix

Thanks a lot

@nikic
Copy link
Member

@nikic nikic commented on 2a535a9 Oct 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Applied in d81eb77.

Please sign in to comment.