On Fri, Jan 22, 2016 at 7:55 PM, H.J. Lu <hongjiu...@intel.com> wrote:
> Without the fix for PR 65656, g++ miscompiles __builtin_constant_p in
> wi::lrshift in wide-int.h. Add a check with PR 65656 testcase to verify
> that C++ __builtin_constant_p works properly.
>
> Tested on x86-64 with working GCC:
>
> gcc/auto-host.h:/* #undef HAVE_WORKING_CXX_BUILTIN_CONSTANT_P */
> prev-gcc/auto-host.h:/* #undef HAVE_WORKING_CXX_BUILTIN_CONSTANT_P */
> stage1-gcc/auto-host.h:#define HAVE_WORKING_CXX_BUILTIN_CONSTANT_P 1
>
> and broken GCC:
>
> gcc/auto-host.h:/* #undef HAVE_WORKING_CXX_BUILTIN_CONSTANT_P */
> prev-gcc/auto-host.h:/* #undef HAVE_WORKING_CXX_BUILTIN_CONSTANT_P */
> stage1-gcc/auto-host.h:/* #undef HAVE_WORKING_CXX_BUILTIN_CONSTANT_P */
>
> Ok for trunk?
I have a hard time seeing how we are "miscompiling"
if (STATIC_CONSTANT_P (xi.precision > HOST_BITS_PER_WIDE_INT)
? xi.len == 1 && xi.val[0] >= 0
: xi.precision <= HOST_BITS_PER_WIDE_INT)
anything that relies on __builtin_constant_p () for sematics is fishy so why
is this not simply an lrshfit implementation bug?
Richard.
> Thanks.
>
> H.J.
> ---
> gcc/
>
> PR c++/69399
> * configure.ac: Check if C++ __builtin_constant_p works
> properly.
> (HAVE_WORKING_CXX_BUILTIN_CONSTANT_P): AC_DEFINE.
> * system.h (STATIC_CONSTANT_P): Use __builtin_constant_p only
> if HAVE_WORKING_CXX_BUILTIN_CONSTANT_P is defined.
> * config.in: Regenerated.
> * configure: Likewise.
>
> gcc/testsuite/
>
> PR c++/69399
> * gcc.dg/torture/pr69399.c: New test.
> ---
> gcc/config.in | 10 ++++++++-
> gcc/configure | 41
> ++++++++++++++++++++++++++++++++--
> gcc/configure.ac | 27 ++++++++++++++++++++++
> gcc/system.h | 2 +-
> gcc/testsuite/gcc.dg/torture/pr69399.c | 21 +++++++++++++++++
> 5 files changed, 97 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/torture/pr69399.c
>
> diff --git a/gcc/config.in b/gcc/config.in
> index 1796e1d..11ebf5c 100644
> --- a/gcc/config.in
> +++ b/gcc/config.in
> @@ -1846,6 +1846,13 @@
> #endif
>
>
> +/* Define this macro if C++ __builtin_constant_p with constexpr does not
> crash
> + with a variable. */
> +#ifndef USED_FOR_TARGET
> +#undef HAVE_WORKING_CXX_BUILTIN_CONSTANT_P
> +#endif
> +
> +
> /* Define to 1 if `fork' works. */
> #ifndef USED_FOR_TARGET
> #undef HAVE_WORKING_FORK
> @@ -1865,7 +1872,8 @@
> #endif
>
>
> -/* Define if your assembler supports .dwsect 0xB0000 */
> +/* Define if your assembler supports AIX debug frame section label reference.
> + */
> #ifndef USED_FOR_TARGET
> #undef HAVE_XCOFF_DWARF_EXTRAS
> #endif
> diff --git a/gcc/configure b/gcc/configure
> index ff646e8..2798231 100755
> --- a/gcc/configure
> +++ b/gcc/configure
> @@ -6534,6 +6534,43 @@ fi
> rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
> fi
>
> +# Check if C++ __builtin_constant_p works properly. Without the fix
> +# for PR 65656, g++ miscompiles __builtin_constant_p in wi::lrshift in
> +# wide-int.h. Add a check with PR 65656 testcase to verify that C++
> +# __builtin_constant_p works properly.
> +if test "$GCC" = yes; then
> + saved_CFLAGS="$CFLAGS"
> + saved_CXXFLAGS="$CXXFLAGS"
> + CFLAGS="$CFLAGS -O -x c++ -std=c++11"
> + CXXFLAGS="$CXXFLAGS -O -x c++ -std=c++11"
> + { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether $CXX
> __builtin_constant_p works with constexpr" >&5
> +$as_echo_n "checking whether $CXX __builtin_constant_p works with
> constexpr... " >&6; }
> + cat confdefs.h - <<_ACEOF >conftest.$ac_ext
> +/* end confdefs.h. */
> +
> +int
> +foo (int argc)
> +{
> + constexpr bool x = __builtin_constant_p(argc);
> + return x ? 1 : 0;
> +}
> +
> +_ACEOF
> +if ac_fn_cxx_try_compile "$LINENO"; then :
> + { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
> +$as_echo "yes" >&6; }
> +
> +$as_echo "#define HAVE_WORKING_CXX_BUILTIN_CONSTANT_P 1" >>confdefs.h
> +
> +else
> + { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
> +$as_echo "no" >&6; }
> +fi
> +rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
> + CFLAGS="$saved_CFLAGS"
> + CXXFLAGS="$saved_CXXFLAGS"
> +fi
> +
> # Check whether compiler is affected by placement new aliasing bug (PR
> 29286).
> # If the host compiler is affected by the bug, and we build with optimization
> # enabled (which happens e.g. when cross-compiling), the pool allocator may
> @@ -18419,7 +18456,7 @@ else
> lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
> lt_status=$lt_dlunknown
> cat > conftest.$ac_ext <<_LT_EOF
> -#line 18422 "configure"
> +#line 18459 "configure"
> #include "confdefs.h"
>
> #if HAVE_DLFCN_H
> @@ -18525,7 +18562,7 @@ else
> lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
> lt_status=$lt_dlunknown
> cat > conftest.$ac_ext <<_LT_EOF
> -#line 18528 "configure"
> +#line 18565 "configure"
> #include "confdefs.h"
>
> #if HAVE_DLFCN_H
> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index 4dc7c10..9791a96 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -416,6 +416,33 @@ struct X<long long> { typedef long long t; };
> ]], [[X<int64_t>::t x;]])],[],[AC_MSG_ERROR([error verifying int64_t uses
> long long])])
> fi
>
> +# Check if C++ __builtin_constant_p works properly. Without the fix
> +# for PR 65656, g++ miscompiles __builtin_constant_p in wi::lrshift in
> +# wide-int.h. Add a check with PR 65656 testcase to verify that C++
> +# __builtin_constant_p works properly.
> +if test "$GCC" = yes; then
> + saved_CFLAGS="$CFLAGS"
> + saved_CXXFLAGS="$CXXFLAGS"
> + CFLAGS="$CFLAGS -O -x c++ -std=c++11"
> + CXXFLAGS="$CXXFLAGS -O -x c++ -std=c++11"
> + AC_MSG_CHECKING(whether $CXX __builtin_constant_p works with constexpr)
> + AC_COMPILE_IFELSE([
> +int
> +foo (int argc)
> +{
> + constexpr bool x = __builtin_constant_p(argc);
> + return x ? 1 : 0;
> +}
> +],
> + [AC_MSG_RESULT([yes])
> + AC_DEFINE(HAVE_WORKING_CXX_BUILTIN_CONSTANT_P, 1,
> +[Define this macro if C++ __builtin_constant_p with constexpr does not
> + crash with a variable.])],
> + [AC_MSG_RESULT([no])])
> + CFLAGS="$saved_CFLAGS"
> + CXXFLAGS="$saved_CXXFLAGS"
> +fi
> +
> # Check whether compiler is affected by placement new aliasing bug (PR
> 29286).
> # If the host compiler is affected by the bug, and we build with optimization
> # enabled (which happens e.g. when cross-compiling), the pool allocator may
> diff --git a/gcc/system.h b/gcc/system.h
> index ba2e963..e57787b 100644
> --- a/gcc/system.h
> +++ b/gcc/system.h
> @@ -729,7 +729,7 @@ extern void fancy_abort (const char *, int, const char *)
> ATTRIBUTE_NORETURN;
> #define gcc_unreachable() (fancy_abort (__FILE__, __LINE__, __FUNCTION__))
> #endif
>
> -#if GCC_VERSION >= 3001
> +#if GCC_VERSION >= 3001 && defined HAVE_WORKING_CXX_BUILTIN_CONSTANT_P
> #define STATIC_CONSTANT_P(X) (__builtin_constant_p (X) && (X))
> #else
> #define STATIC_CONSTANT_P(X) (false && (X))
> diff --git a/gcc/testsuite/gcc.dg/torture/pr69399.c
> b/gcc/testsuite/gcc.dg/torture/pr69399.c
> new file mode 100644
> index 0000000..3e17169
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/torture/pr69399.c
> @@ -0,0 +1,21 @@
> +/* { dg-do run { target int128 } } */
> +
> +typedef __UINT64_TYPE__ u64;
> +typedef unsigned __int128 u128;
> +
> +static unsigned __attribute__((noinline, noclone))
> +foo(u64 u)
> +{
> + u128 v = u | 0xffffff81;
> + v >>= 64;
> + return v;
> +}
> +
> +int
> +main()
> +{
> + unsigned x = foo(27);
> + if (x != 0)
> + __builtin_abort();
> + return 0;
> +}
> --
> 2.5.0
>
