google / highway

Performance-portable, length-agnostic SIMD with runtime dispatch
Apache License 2.0
4.22k stars 319 forks source link

Errors building 1.0.0 for multiple architectures #938

Closed alucryd closed 2 years ago

alucryd commented 2 years ago

Hi guys, I'm trying to cross-compile highway for a lot of achitectures, but I'm running into a lot of seemingly "missing" parentheses, which seems odd because it builds just fine for half of them already. Using mostly crosstool-ng toolchains for all of them, GCC 10, but also mingw for windows and ndk for android. Having troubles with linux armv5, armv7, ppc32 and x86 targets, below is the full log, same error for all of them: https://paste.xinu.at/ZEKmexD183AkbI72/

Any ideas?

jan-wassenberg commented 2 years ago

Hi, this seems to be something like https://github.com/andrew-d/rough-auditing-tool-for-security/issues/9, where the checker/warning doesn't understand the inttypes.h mechanism (a macro that expands to the actual format string). In this file we could implement a (u)int64 -> string conversion but unfortunately inttypes.h is used more widely.

Regrettable though it is, looks like the best option might be to disable -Wformat-extra-args and -Wformat= for gcc builds?

alucryd commented 2 years ago

Unfortunately passing -Wno-format -Wno-format-extra-args only takes care of the warnings, the errors remain.

ninja: Entering directory `build'
[1/10] Building CXX object CMakeFiles/hwy.dir/hwy/print.cc.o
FAILED: CMakeFiles/hwy.dir/hwy/print.cc.o 
/home/embybuilder/Buildbot/armv5/toolchain/bin/arm-emby-linux-gnueabi-g++ --sysroot=/home/embybuilder/Buildbot/armv5/toolchain/arm-emby-linux-gnueabi/sysroot -DHWY_STATIC_DEFINE -I/home/embybuilder/Buildbot/armv5/libhighway-armv5/build -march=armv5te -mtune=arm9e -mfloat-abi=soft -marm -Os -pipe -fno-plt -I/home/embybuilder/Buildbot/armv5/staging/include -Wno-format -Wno-format-extra-args -O3 -DNDEBUG -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -Wno-builtin-macro-redefined -D__DATE__=\"redacted\" -D__TIMESTAMP__=\"redacted\" -D__TIME__=\"redacted\" -fmerge-all-constants -Wall -Wextra -Wconversion -Wsign-conversion -Wvla -Wnon-virtual-dtor -fmath-errno -fno-exceptions -MD -MT CMakeFiles/hwy.dir/hwy/print.cc.o -MF CMakeFiles/hwy.dir/hwy/print.cc.o.d -o CMakeFiles/hwy.dir/hwy/print.cc.o -c /home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/print.cc
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/print.cc: In function 'void hwy::detail::ToString(const hwy::detail::TypeInfo&, const void*, char*)':
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/print.cc:74:35: error: expected ')' before 'PRIi64'
   74 |       snprintf(string100, 100, "%" PRIi64 "", value);  // NOLINT
      |               ~                   ^~~~~~~
      |                                   )
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/print.cc:78:35: error: expected ')' before 'PRIu64'
   78 |       snprintf(string100, 100, "%" PRIu64 "", value);  // NOLINT
      |               ~                   ^~~~~~~
      |                                   )
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/print.cc: In function 'void hwy::detail::PrintArray(const hwy::detail::TypeInfo&, const char*, const void*, size_t, size_t, size_t)':
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/print.cc:94:29: error: expected ')' before 'PRIu64'
   94 |   fprintf(stderr, "%s %s [%" PRIu64 "+ ->]:\n  ", type_name, caption,
      |          ~                  ^~~~~~~
      |                             )
[2/10] Building CXX object CMakeFiles/hwy.dir/hwy/targets.cc.o
[3/10] Building CXX object CMakeFiles/hwy.dir/hwy/per_target.cc.o
[4/10] Building CXX object CMakeFiles/hwy.dir/hwy/aligned_allocator.cc.o
[5/10] Building CXX object CMakeFiles/hwy_list_targets.dir/hwy/tests/list_targets.cc.o
[6/10] Building CXX object CMakeFiles/hwy_test.dir/hwy/tests/test_util.cc.o
FAILED: CMakeFiles/hwy_test.dir/hwy/tests/test_util.cc.o 
/home/embybuilder/Buildbot/armv5/toolchain/bin/arm-emby-linux-gnueabi-g++ --sysroot=/home/embybuilder/Buildbot/armv5/toolchain/arm-emby-linux-gnueabi/sysroot -DHWY_STATIC_DEFINE -I/home/embybuilder/Buildbot/armv5/libhighway-armv5/build -march=armv5te -mtune=arm9e -mfloat-abi=soft -marm -Os -pipe -fno-plt -I/home/embybuilder/Buildbot/armv5/staging/include -Wno-format -Wno-format-extra-args -O3 -DNDEBUG -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -Wno-builtin-macro-redefined -D__DATE__=\"redacted\" -D__TIMESTAMP__=\"redacted\" -D__TIME__=\"redacted\" -fmerge-all-constants -Wall -Wextra -Wconversion -Wsign-conversion -Wvla -Wnon-virtual-dtor -fmath-errno -fno-exceptions -MD -MT CMakeFiles/hwy_test.dir/hwy/tests/test_util.cc.o -MF CMakeFiles/hwy_test.dir/hwy/tests/test_util.cc.o.d -o CMakeFiles/hwy_test.dir/hwy/tests/test_util.cc.o -c /home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/tests/test_util.cc
In file included from /home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/tests/test_util.h:28,
                 from /home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/tests/test_util.cc:16:
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/tests/test_util.cc: In function 'bool hwy::detail::IsEqual(const hwy::detail::TypeInfo&, const void*, const void*)':
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/tests/test_util.cc:74:41: error: expected ')' before 'PRIu64'
   74 |     HWY_ABORT("Unexpected float size %" PRIu64 "\n",
      |                                         ^~~~~~
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/base.h:164:36: note: in definition of macro 'HWY_ABORT'
  164 |   ::hwy::Abort(__FILE__, __LINE__, format, ##__VA_ARGS__)
      |                                    ^~~~~~
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/base.h:164:15: note: to match this '('
  164 |   ::hwy::Abort(__FILE__, __LINE__, format, ##__VA_ARGS__)
      |               ^
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/tests/test_util.cc:74:5: note: in expansion of macro 'HWY_ABORT'
   74 |     HWY_ABORT("Unexpected float size %" PRIu64 "\n",
      |     ^~~~~~~~~
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/tests/test_util.cc: In function 'void hwy::detail::PrintMismatchAndAbort(const hwy::detail::TypeInfo&, const void*, const void*, const char*, const char*, int, size_t, size_t)':
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/tests/test_util.cc:91:19: error: expected ')' before 'PRIu64'
   91 |         "%s, %sx%" PRIu64 " lane %" PRIu64
      |                   ^~~~~~~
      |                   )
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/tests/test_util.cc:90:8: note: to match this '('
   90 |   Abort(filename, line,
      |        ^
[7/10] Building CXX object CMakeFiles/hwy.dir/hwy/nanobenchmark.cc.o
FAILED: CMakeFiles/hwy.dir/hwy/nanobenchmark.cc.o 
/home/embybuilder/Buildbot/armv5/toolchain/bin/arm-emby-linux-gnueabi-g++ --sysroot=/home/embybuilder/Buildbot/armv5/toolchain/arm-emby-linux-gnueabi/sysroot -DHWY_STATIC_DEFINE -I/home/embybuilder/Buildbot/armv5/libhighway-armv5/build -march=armv5te -mtune=arm9e -mfloat-abi=soft -marm -Os -pipe -fno-plt -I/home/embybuilder/Buildbot/armv5/staging/include -Wno-format -Wno-format-extra-args -O3 -DNDEBUG -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -Wno-builtin-macro-redefined -D__DATE__=\"redacted\" -D__TIMESTAMP__=\"redacted\" -D__TIME__=\"redacted\" -fmerge-all-constants -Wall -Wextra -Wconversion -Wsign-conversion -Wvla -Wnon-virtual-dtor -fmath-errno -fno-exceptions -MD -MT CMakeFiles/hwy.dir/hwy/nanobenchmark.cc.o -MF CMakeFiles/hwy.dir/hwy/nanobenchmark.cc.o.d -o CMakeFiles/hwy.dir/hwy/nanobenchmark.cc.o -c /home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/nanobenchmark.cc
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/nanobenchmark.cc: In function 'hwy::{anonymous}::timer::Ticks hwy::{anonymous}::SampleUntilStable(double, double*, const hwy::Params&, const Lambda&)':
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/nanobenchmark.cc:537:20: error: expected ')' before 'PRIu64'
  537 |         printf("%6" PRIu64 " samples => %5" PRIu64 " (abs_mad=%4" PRIu64
      |               ~    ^~~~~~~
      |                    )
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/nanobenchmark.cc:548:69: error: expected ')' before 'PRIu64'
  548 |     printf("WARNING: rel_mad=%4.2f%% still exceeds %4.2f%% after %6" PRIu64
      |           ~                                                         ^~~~~~~
      |                                                                     )
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/nanobenchmark.cc: In function 'size_t hwy::{anonymous}::NumSkip(hwy::Func, const uint8_t*, const InputVec&, const hwy::Params&)':
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/nanobenchmark.cc:588:19: error: expected ')' before 'PRIu64'
  588 |     printf("res=%" PRIu64 " max_skip=%" PRIu64 " min_dur=%" PRIu64
      |           ~       ^~~~~~~
      |                   )
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/nanobenchmark.cc: In function 'size_t hwy::Measure(hwy::Func, const uint8_t*, const FuncInput*, size_t, hwy::Result*, const hwy::Params&)':
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/nanobenchmark.cc:720:53: error: expected ')' before 'PRIu64'
  720 |     fprintf(stderr, "Measurement failed: overhead %" PRIu64 " < %" PRIu64 "\n",
      |            ~                                        ^~~~~~~
      |                                                     )
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/nanobenchmark.cc:727:24: error: expected ')' before 'PRIu64'
  727 |     printf("#inputs=%5" PRIu64 ",%5" PRIu64 " overhead=%5" PRIu64 ",%5" PRIu64
      |           ~            ^~~~~~~
      |                        )
/home/embybuilder/Buildbot/armv5/libhighway-armv5/build/hwy/nanobenchmark.cc:744:52: error: expected ')' before 'PRIu64'
  744 |       fprintf(stderr, "Measurement failed: total %" PRIu64 " < %" PRIu64 "\n",
      |              ~                                     ^~~~~~~
      |                                                    )
ninja: build stopped: subcommand failed.
jan-wassenberg commented 2 years ago

hm, this is difficult to understand. I've double-checked the PRIi64 documentation and that we are including inttypes.h.

I suspect that PRIi64 is somehow not getting defined. Let's verify this: does this, when inserted before the failing line, raise an error?

#ifndef PRIi64
#error "not defined"
#endif

One unlikely idea: does it help to change the include to #include <cinttypes>?

alucryd commented 2 years ago

Hmm, PRIi64 is defined in all toolchains as # define PRIi64 __PRI64_PREFIX "i".

Comparing inttypes.h from a working and non-working toolchain doesn't reveal much difference:

diff -rupN armv5/toolchain/arm-emby-linux-gnueabi/sysroot/usr/include/inttypes.h x64/toolchain/x86_64-emby-linux-gnu/sysroot/usr/include/inttypes.h
--- armv5/toolchain/arm-emby-linux-gnueabi/sysroot/usr/include/inttypes.h   2022-07-15 14:28:34.626986872 +0000
+++ x64/toolchain/x86_64-emby-linux-gnu/sysroot/usr/include/inttypes.h  2022-05-27 20:24:59.337233803 +0000
@@ -1,4 +1,4 @@
-/* Copyright (C) 1997-2001, 2004, 2007, 2012 Free Software Foundation, Inc.
+/* Copyright (C) 1997-2018 Free Software Foundation, Inc.
    This file is part of the GNU C Library.

    The GNU C Library is free software; you can redistribute it and/or
@@ -40,11 +40,6 @@ typedef wchar_t __gwchar_t;
 # define ____gwchar_t_defined  1
 #endif

-
-/* The ISO C99 standard specifies that these macros must only be
-   defined if explicitly requested.  */
-#if !defined __cplusplus || defined __STDC_FORMAT_MACROS
-
 # if __WORDSIZE == 64
 #  define __PRI64_PREFIX   "l"
 #  define __PRIPTR_PREFIX  "l"
@@ -267,8 +262,6 @@ typedef wchar_t __gwchar_t;
 # define SCNuPTR   __PRIPTR_PREFIX "u"
 # define SCNxPTR   __PRIPTR_PREFIX "x"

-#endif /* C++ && format macros */
-

 __BEGIN_DECLS

@@ -286,8 +279,8 @@ typedef struct
 /* We have to define the `uintmax_t' type using `lldiv_t'.  */
 typedef struct
   {
-    long long int quot;        /* Quotient.  */
-    long long int rem;     /* Remainder.  */
+    __extension__ long long int quot;  /* Quotient.  */
+    __extension__ long long int rem;   /* Remainder.  */
   } imaxdiv_t;

 #endif

I can find a GCC header file that redefines PRIi64 though, lib/gcc/arm-emby-linux-gnueabi/10.3.0/plugin/include/hwint.h contains the following block in every toolchain:

/* Provide C99 <inttypes.h> style format definitions for 64bits.  */
#ifndef HAVE_INTTYPES_H
#if INT64_T_IS_LONG
# define GCC_PRI64 HOST_LONG_FORMAT
#else
# define GCC_PRI64 HOST_LONG_LONG_FORMAT
#endif
#undef PRId64
#define PRId64 GCC_PRI64 "d"
#undef PRIi64
#define PRIi64 GCC_PRI64 "i"
#undef PRIo64
#define PRIo64 GCC_PRI64 "o"
#undef PRIu64
#define PRIu64 GCC_PRI64 "u"
#undef PRIx64
#define PRIx64 GCC_PRI64 "x"
#undef PRIX64
#define PRIX64 GCC_PRI64 "X"
#endif

Oh, and I don't have any cinttypes.h header in any toolchain.

alucryd commented 2 years ago

Building a simple hello world appears to work, PRIi64 is correctly defined.

#include <stdio.h>
#include <inttypes.h>

#ifndef PRIi64
#error "not defined"
#endif

int main()
{
    printf("Hello World");
    return 0;
}
./arm-emby-linux-gnueabi-gcc -Os test.c -o test
readelf -h test
ELF Header:
  Magic:   7f 45 4c 46 01 01 01 00 00 00 00 00 00 00 00 00 
  Class:                             ELF32
  Data:                              2's complement, little endian
  Version:                           1 (current)
  OS/ABI:                            UNIX - System V
  ABI Version:                       0
  Type:                              EXEC (Executable file)
  Machine:                           ARM
  Version:                           0x1
  Entry point address:               0x1030c
  Start of program headers:          52 (bytes into file)
  Start of section headers:          7868 (bytes into file)
  Flags:                             0x5000200, Version5 EABI, soft-float ABI
  Size of this header:               52 (bytes)
  Size of program headers:           32 (bytes)
  Number of program headers:         8
  Size of section headers:           40 (bytes)
  Number of section headers:         37
  Section header string table index: 34
jan-wassenberg commented 2 years ago

Ah, nice idea to diff the headers. I think defining __STDC_FORMAT_MACROS would solve this. We can add that to our code if you confirm that works.

See also https://github.com/WAVM/WAVM/pull/266.

alucryd commented 2 years ago

1.0.1 builds like a charm on every architecture, thank you very much!

jan-wassenberg commented 2 years ago

Thanks for letting us know :)