New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 746420 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug

Blocking:
issue 672489


Participants' hotlists:
Chromium-Packagers


Sign in to add a comment

ClampedNumeric optimizations fail to compile with GCC 6

Project Member Reported by phajdan.jr@chromium.org, Jul 19 2017

Issue description

Chrome Version: 61.0.3159.5 (source code as of that tag)
OS: Linux (GCC 6)

I hit the following compile error:

FAILED: base/message_loop/message_loop.o
x86_64-pc-linux-gnu-g++ -MMD -MF base/message_loop/message_loop.o.d  -I/var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/out_bootstrap/gen -I/var/tmp/portag
e/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5 -O2 -pipe -march=native -DNO_TCMALLOC -D__STDC_FORMAT_MACROS -O2 -g0 -D_FILE_OFFSET_BITS=64 -D__STDC_CONSTANT_MACROS -D__
STDC_FORMAT_MACROS -pthread -pipe -fno-exceptions -O2 -pipe -march=native -fno-delete-null-pointer-checks -std=c++11 -Wno-c++11-narrowing -c /var/tmp/portage/www-client/chromium-61.
0.3159.5/work/chromium-61.0.3159.5/base/message_loop/message_loop.cc -o base/message_loop/message_loop.o
In file included from /var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/numerics/safe_math_shared_impl.h:26:0,
                 from /var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/numerics/checked_math_impl.h:18,
                 from /var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/numerics/checked_math.h:13,
                 from /var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/numerics/safe_math.h:8,
                 from /var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/time/time.h:63,
                 from /var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/threading/platform_thread.h:16,
                 from /var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/synchronization/lock.h:12,
                 from /var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/sequence_checker_impl.h:13,
                 from /var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/sequence_checker.h:10,
                 from /var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/memory/ref_counted.h:19,
                 from /var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/message_loop/message_loop.h:17,
                 from /var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/message_loop/message_loop.cc:5:
/var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/numerics/safe_math_clang_gcc_impl.h: In function ‘int8_t base::internal::ClampedNegate(uint8_t)’:
/var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/numerics/safe_math_clang_gcc_impl.h:132:45: error: ‘__builtin_subcb’ was not declared in this scope
   return __builtin_subcb(0, value, 0, &carry) + carry;
                                             ^
/var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/numerics/safe_math_clang_gcc_impl.h: In function ‘int16_t base::internal::ClampedNegate(uint16_t)’:
/var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/numerics/safe_math_clang_gcc_impl.h:141:45: error: ‘__builtin_subcs’ was not declared in this scope
   return __builtin_subcs(0, value, 0, &carry) + carry;
                                             ^
/var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/numerics/safe_math_clang_gcc_impl.h: In function ‘int32_t base::internal::ClampedNegate(uint32_t)’:
/var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/numerics/safe_math_clang_gcc_impl.h:150:44: error: ‘__builtin_subc’ was not declared in this scope
   return __builtin_subc(0, value, 0, &carry) + carry;
                                            ^
/var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/numerics/safe_math_clang_gcc_impl.h: In function ‘int64_t base::internal::ClampedNegate(uint64_t)’:
/var/tmp/portage/www-client/chromium-61.0.3159.5/work/chromium-61.0.3159.5/base/numerics/safe_math_clang_gcc_impl.h:161:45: error: ‘__builtin_subcl’ was not declared in this scope
   return __builtin_subcl(0, value, 0, &carry) + carry;
                                             ^

I traced this to https://chromium-review.googlesource.com/c/571404/ . __builtin_subcb and others seem to be clang-specific. They've been added in http://llvm.org/viewvc/llvm-project?view=revision&revision=184227 .

Why wasn't this easily caught in Chromium? https://chromium.googlesource.com/chromium/src/+/f36960a8d9bc58ac7ea0b0ea6e986bcfdf5e08ef/base/numerics/safe_math_shared_impl.h#25 only enables optimizations for clang (where the extensions work) and GCC 5 or higher. On Ubuntu Trusty gcc version is 4.8.4. This is what I'm using that reproduces the error:

$ gcc -v
Using built-in specs.
COLLECT_GCC=/usr/x86_64-pc-linux-gnu/gcc-bin/6.3.0/gcc
COLLECT_LTO_WRAPPER=/usr/libexec/gcc/x86_64-pc-linux-gnu/6.3.0/lto-wrapper
Target: x86_64-pc-linux-gnu
Configured with: /var/tmp/portage/sys-devel/gcc-6.3.0/work/gcc-6.3.0/configure --host=x86_64-pc-linux-gnu --build=x86_64-pc-linux-gnu --prefix=/usr --bindir=/usr/x86_64-pc-linux-gnu/gcc-bin/6.3.0 --includedir=/usr/lib/gcc/x86_64-pc-linux-gnu/6.3.0/include --datadir=/usr/share/gcc-data/x86_64-pc-linux-gnu/6.3.0 --mandir=/usr/share/gcc-data/x86_64-pc-linux-gnu/6.3.0/man --infodir=/usr/share/gcc-data/x86_64-pc-linux-gnu/6.3.0/info --with-gxx-include-dir=/usr/lib/gcc/x86_64-pc-linux-gnu/6.3.0/include/g++-v6 --with-python-dir=/share/gcc-data/x86_64-pc-linux-gnu/6.3.0/python --enable-languages=c,c++,fortran --enable-obsolete --enable-secureplt --disable-werror --with-system-zlib --enable-nls --without-included-gettext --enable-checking=release --with-bugurl=https://bugs.gentoo.org/ --with-pkgversion='Gentoo 6.3.0 p1.0' --disable-esp --enable-libstdcxx-time --enable-shared --enable-threads=posix --enable-__cxa_atexit --enable-clocale=gnu --enable-multilib --with-multilib-list=m32,m64 --disable-altivec --disable-fixed-point --enable-targets=all --disable-libgcj --enable-libgomp --disable-libmudflap --disable-libssp --disable-libcilkrts --disable-libmpx --enable-vtable-verify --enable-libvtv --enable-lto --without-isl --enable-libsanitizer --disable-default-pie --enable-default-ssp
Thread model: posix
gcc version 6.3.0 (Gentoo 6.3.0 p1.0)

Her's a patch I used as workaround. For easy repro on Ubuntu, you could change the #ifdef to also enable optimizations for GCC 4.

diff --git a/base/numerics/safe_math_shared_impl.h b/base/numerics/safe_math_shared_impl.h
index 99f230ce7e9a..de2415d402f5 100644
--- a/base/numerics/safe_math_shared_impl.h
+++ b/base/numerics/safe_math_shared_impl.h
@@ -21,8 +21,7 @@
 #if !defined(__native_client__) &&                         \
     ((defined(__clang__) &&                                \
       ((__clang_major__ > 3) ||                            \
-       (__clang_major__ == 3 && __clang_minor__ >= 4))) || \
-     (defined(__GNUC__) && __GNUC__ >= 5))
+       (__clang_major__ == 3 && __clang_minor__ >= 4))))
 #include "base/numerics/safe_math_clang_gcc_impl.h"
 #define BASE_HAS_OPTIMIZED_SAFE_MATH (1)
 #else

The conclusion from this is that probably above #ifdef is not precise enough with https://chromium-review.googlesource.com/c/571404/ . The builtins it uses may need a different condition.
 

Comment 1 by jsc...@chromium.org, Jul 19 2017

If you want to post a CL removing the optimization path for GCC I'm happy to lgtm it. Because, I don't really have the capability to test on GCC anymore.
Project Member

Comment 2 by bugdroid1@chromium.org, Jul 21 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cbe6845263215e0f3981c2a4c7937dadb14bef0d

commit cbe6845263215e0f3981c2a4c7937dadb14bef0d
Author: Justin Schuh <jschuh@chromium.org>
Date: Fri Jul 21 12:32:31 2017

Make base/numerics build with GCC

Disables the __builtin_adc intrinsics on GCC.

Bug:  746420 
Change-Id: I1b16eaba85ca93c2d04dc25755565cd9e0386320
Reviewed-on: https://chromium-review.googlesource.com/580787
Reviewed-by: Paweł Hajdan Jr. <phajdan.jr@chromium.org>
Commit-Queue: Justin Schuh <jschuh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#488636}
[modify] https://crrev.com/cbe6845263215e0f3981c2a4c7937dadb14bef0d/base/numerics/safe_math_clang_gcc_impl.h

Comment 3 by jsc...@chromium.org, Jul 21 2017

Cc: -jsc...@chromium.org
Owner: jsc...@chromium.org
Status: Fixed (was: Untriaged)

Sign in to add a comment