| Message ID | 20160122185533.GA3430@intel.com |
|---|---|
| State | New |
| Headers | show |
On Fri, Jan 22, 2016 at 7:55 PM, H.J. Lu <hongjiu.lu@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 >
On Mon, Jan 25, 2016 at 4:40 AM, Richard Biener <richard.guenther@gmail.com> wrote: > On Fri, Jan 22, 2016 at 7:55 PM, H.J. Lu <hongjiu.lu@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? > We hit this via: Breakpoint 1, wi::lrshift<generic_wide_int<fixed_wide_int_storage<192> >, generic_wide_int<fixed_wide_int_storage<192> > > (x=..., y=...) at /export/gnu/import/git/sources/gcc-release/gcc/wide-int.h:2898 2898 val[0] = xi.to_uhwi () >> shift; (gdb) bt #0 wi::lrshift<generic_wide_int<fixed_wide_int_storage<192> >, generic_wide_int<fixed_wide_int_storage<192> > > (x=..., y=...) at /export/gnu/import/git/sources/gcc-release/gcc/wide-int.h:2898 #1 0x00000000009e7bbe in wi::rshift<generic_wide_int<fixed_wide_int_storage<192> >, generic_wide_int<fixed_wide_int_storage<192> > > (sgn=<optimized out>, y=..., x=...) at /export/gnu/import/git/sources/gcc-release/gcc/wide-int.h:2947 #2 bit_value_binop_1 (code=code@entry=RSHIFT_EXPR, type=type@entry=0x7fffefe82dc8, val=val@entry=0x7fffffffd7c0, mask=mask@entry=0x7fffffffd790, r1type=0x7fffefe82dc8, r1val=..., r1mask=..., r2type=0x7fffefd6b690, r2val=..., r2mask=...) at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-ccp.c:1348 #3 0x00000000009e9e7b in bit_value_binop (code=code@entry=RSHIFT_EXPR, type=0x7fffefe82dc8, rhs1=rhs1@entry=0x7fffefd71708, rhs2=<optimized out>) at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-ccp.c:1549 #4 0x00000000009eb520 in evaluate_stmt (stmt=stmt@entry=0x7fffefe9a160) at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-ccp.c:1785 #5 0x00000000009ec8d2 in visit_assignment (stmt=stmt@entry=0x7fffefe9a160, output_p=output_p@entry=0x7fffffffdba0) at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-ccp.c:2258 #6 0x00000000009ec9c2 in ccp_visit_stmt (stmt=0x7fffefe9a160, taken_edge_p=0x7fffffffdba8, output_p=0x7fffffffdba0) at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-ccp.c:2336 ---Type <return> to continue, or q <return> to quit--- #7 0x0000000000a4efcf in simulate_stmt (stmt=stmt@entry=0x7fffefe9a160) at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-propagate.c:348 #8 0x0000000000a50f79 in simulate_block (block=<optimized out>) at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-propagate.c:471 #9 ssa_propagate ( visit_stmt=visit_stmt@entry=0x9ec937 <ccp_visit_stmt(gimple, edge*, tree*)>, visit_phi=visit_phi@entry=0x9e6aa5 <ccp_visit_phi_node(gphi*)>) at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-propagate.c:888 #10 0x00000000009e6295 in do_ssa_ccp () at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-ccp.c:2382 #11 (anonymous namespace)::pass_ccp::execute (this=<optimized out>) at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-ccp.c:2415 #12 0x000000000089ca0c in execute_one_pass (pass=pass@entry=0x19b4bf0) at /export/gnu/import/git/sources/gcc-release/gcc/passes.c:2330 #13 0x000000000089cd62 in execute_pass_list_1 (pass=0x19b4bf0) at /export/gnu/import/git/sources/gcc-release/gcc/passes.c:2382 #14 0x000000000089cd7f in execute_pass_list_1 (pass=0x19b4a70, pass@entry=0x19b48f0) at /export/gnu/import/git/sources/gcc-release/gcc/passes.c:2383 #15 0x000000000089cd9c in execute_pass_list (fn=0x7fffefe98000, pass=0x19b48f0) at /export/gnu/import/git/sources/gcc-release/gcc/passes.c:2393 #16 0x000000000089ba57 in do_per_function_toporder ( callback=callback@entry=0x89cd83 <execute_pass_list(function*, opt_pass*)>, ---Type <return> to continue, or q <return> to quit--- data=0x19b48f0) at /export/gnu/import/git/sources/gcc-release/gcc/passes.c:1728 #17 0x000000000089d3e3 in execute_ipa_pass_list (pass=0x19b4890) at /export/gnu/import/git/sources/gcc-release/gcc/passes.c:2736 #18 0x000000000066f3ac in ipa_passes () at /export/gnu/import/git/sources/gcc-release/gcc/cgraphunit.c:2172 #19 symbol_table::compile (this=this@entry=0x7fffefd6b000) at /export/gnu/import/git/sources/gcc-release/gcc/cgraphunit.c:2313 #20 0x0000000000670be5 in symbol_table::finalize_compilation_unit ( this=0x7fffefd6b000) at /export/gnu/import/git/sources/gcc-release/gcc/cgraphunit.c:2462 #21 0x000000000058ea41 in c_write_global_declarations () at /export/gnu/import/git/sources/gcc-release/gcc/c/c-decl.c:10822 #22 0x0000000000939509 in compile_file () at /export/gnu/import/git/sources/gcc-release/gcc/toplev.c:613 #23 0x000000000093b3c4 in do_compile () at /export/gnu/import/git/sources/gcc-release/gcc/toplev.c:2067 #24 toplev::main (this=this@entry=0x7fffffffdf60, argc=argc@entry=15, argv=argv@entry=0x7fffffffe068) at /export/gnu/import/git/sources/gcc-release/gcc/toplev.c:2165 #25 0x0000000000f50ab7 in main (argc=15, argv=0x7fffffffe068) at /export/gnu/import/git/sources/gcc-release/gcc/main.c:39 (gdb) p x $2 = (const generic_wide_int<fixed_wide_int_storage<192> > &) @0x7fffffffd670: {<fixed_wide_int_storage<192>> = {val = {4294967169, 0, 140737217214936, 92}, len = 1}, static is_sign_extended = <optimized out>} (gdb) p y $3 = (const generic_wide_int<fixed_wide_int_storage<192> > &) @0x7fffffffd640: {<fixed_wide_int_storage<192>> = {val = {64, 824633720833, -4294967168, 140737218308160}, len = 1}, static is_sign_extended = <optimized out>} (gdb) p xi $3 = {<wide_int_ref_storage<true>> = {<wi::storage_ref> = { val = 0x7fffffffcfe0, len = 2, precision = 192}, scratch = {64, 8589921072}}, static is_sign_extended = <optimized out>} (gdb) p yi $4 = {<wide_int_ref_storage<true>> = {<wi::storage_ref> = { val = 0x7fffffffcbc0, len = 1, precision = 192}, scratch = { 140737488344768, 140737488343008}}, static is_sign_extended = <optimized out>} (gdb) Somehow PR 65656 miscompiled: 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) which turned the expression into true and hit val[0] = xi.to_uhwi () >> shift; result.set_len (1);
On Mon, Jan 25, 2016 at 5:25 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Mon, Jan 25, 2016 at 4:40 AM, Richard Biener > <richard.guenther@gmail.com> wrote: >> On Fri, Jan 22, 2016 at 7:55 PM, H.J. Lu <hongjiu.lu@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? >> > > > We hit this via: > > Breakpoint 1, wi::lrshift<generic_wide_int<fixed_wide_int_storage<192> >>, generic_wide_int<fixed_wide_int_storage<192> > > (x=..., y=...) > at /export/gnu/import/git/sources/gcc-release/gcc/wide-int.h:2898 > 2898 val[0] = xi.to_uhwi () >> shift; > (gdb) bt > #0 wi::lrshift<generic_wide_int<fixed_wide_int_storage<192> >, > generic_wide_int<fixed_wide_int_storage<192> > > (x=..., y=...) > at /export/gnu/import/git/sources/gcc-release/gcc/wide-int.h:2898 > #1 0x00000000009e7bbe in > wi::rshift<generic_wide_int<fixed_wide_int_storage<192> >, > generic_wide_int<fixed_wide_int_storage<192> > > (sgn=<optimized out>, > y=..., x=...) > at /export/gnu/import/git/sources/gcc-release/gcc/wide-int.h:2947 > #2 bit_value_binop_1 (code=code@entry=RSHIFT_EXPR, > type=type@entry=0x7fffefe82dc8, val=val@entry=0x7fffffffd7c0, > mask=mask@entry=0x7fffffffd790, r1type=0x7fffefe82dc8, r1val=..., > r1mask=..., r2type=0x7fffefd6b690, r2val=..., r2mask=...) > at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-ccp.c:1348 > #3 0x00000000009e9e7b in bit_value_binop (code=code@entry=RSHIFT_EXPR, > type=0x7fffefe82dc8, rhs1=rhs1@entry=0x7fffefd71708, rhs2=<optimized out>) > at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-ccp.c:1549 > #4 0x00000000009eb520 in evaluate_stmt (stmt=stmt@entry=0x7fffefe9a160) > at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-ccp.c:1785 > #5 0x00000000009ec8d2 in visit_assignment (stmt=stmt@entry=0x7fffefe9a160, > output_p=output_p@entry=0x7fffffffdba0) > at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-ccp.c:2258 > #6 0x00000000009ec9c2 in ccp_visit_stmt (stmt=0x7fffefe9a160, > taken_edge_p=0x7fffffffdba8, output_p=0x7fffffffdba0) > at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-ccp.c:2336 > ---Type <return> to continue, or q <return> to quit--- > #7 0x0000000000a4efcf in simulate_stmt (stmt=stmt@entry=0x7fffefe9a160) > at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-propagate.c:348 > #8 0x0000000000a50f79 in simulate_block (block=<optimized out>) > at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-propagate.c:471 > #9 ssa_propagate ( > visit_stmt=visit_stmt@entry=0x9ec937 <ccp_visit_stmt(gimple, > edge*, tree*)>, visit_phi=visit_phi@entry=0x9e6aa5 > <ccp_visit_phi_node(gphi*)>) > at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-propagate.c:888 > #10 0x00000000009e6295 in do_ssa_ccp () > at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-ccp.c:2382 > #11 (anonymous namespace)::pass_ccp::execute (this=<optimized out>) > at /export/gnu/import/git/sources/gcc-release/gcc/tree-ssa-ccp.c:2415 > #12 0x000000000089ca0c in execute_one_pass (pass=pass@entry=0x19b4bf0) > at /export/gnu/import/git/sources/gcc-release/gcc/passes.c:2330 > #13 0x000000000089cd62 in execute_pass_list_1 (pass=0x19b4bf0) > at /export/gnu/import/git/sources/gcc-release/gcc/passes.c:2382 > #14 0x000000000089cd7f in execute_pass_list_1 (pass=0x19b4a70, > pass@entry=0x19b48f0) > at /export/gnu/import/git/sources/gcc-release/gcc/passes.c:2383 > #15 0x000000000089cd9c in execute_pass_list (fn=0x7fffefe98000, pass=0x19b48f0) > at /export/gnu/import/git/sources/gcc-release/gcc/passes.c:2393 > #16 0x000000000089ba57 in do_per_function_toporder ( > callback=callback@entry=0x89cd83 <execute_pass_list(function*, > opt_pass*)>, ---Type <return> to continue, or q <return> to quit--- > data=0x19b48f0) at /export/gnu/import/git/sources/gcc-release/gcc/passes.c:1728 > #17 0x000000000089d3e3 in execute_ipa_pass_list (pass=0x19b4890) > at /export/gnu/import/git/sources/gcc-release/gcc/passes.c:2736 > #18 0x000000000066f3ac in ipa_passes () > at /export/gnu/import/git/sources/gcc-release/gcc/cgraphunit.c:2172 > #19 symbol_table::compile (this=this@entry=0x7fffefd6b000) > at /export/gnu/import/git/sources/gcc-release/gcc/cgraphunit.c:2313 > #20 0x0000000000670be5 in symbol_table::finalize_compilation_unit ( > this=0x7fffefd6b000) > at /export/gnu/import/git/sources/gcc-release/gcc/cgraphunit.c:2462 > #21 0x000000000058ea41 in c_write_global_declarations () > at /export/gnu/import/git/sources/gcc-release/gcc/c/c-decl.c:10822 > #22 0x0000000000939509 in compile_file () > at /export/gnu/import/git/sources/gcc-release/gcc/toplev.c:613 > #23 0x000000000093b3c4 in do_compile () > at /export/gnu/import/git/sources/gcc-release/gcc/toplev.c:2067 > #24 toplev::main (this=this@entry=0x7fffffffdf60, argc=argc@entry=15, > argv=argv@entry=0x7fffffffe068) > at /export/gnu/import/git/sources/gcc-release/gcc/toplev.c:2165 > #25 0x0000000000f50ab7 in main (argc=15, argv=0x7fffffffe068) > at /export/gnu/import/git/sources/gcc-release/gcc/main.c:39 > (gdb) p x > $2 = (const generic_wide_int<fixed_wide_int_storage<192> > &) > @0x7fffffffd670: {<fixed_wide_int_storage<192>> = {val = {4294967169, > 0, 140737217214936, 92}, > len = 1}, static is_sign_extended = <optimized out>} > (gdb) p y > $3 = (const generic_wide_int<fixed_wide_int_storage<192> > &) > @0x7fffffffd640: {<fixed_wide_int_storage<192>> = {val = {64, > 824633720833, -4294967168, > 140737218308160}, len = 1}, static is_sign_extended = <optimized out>} > (gdb) p xi > $3 = {<wide_int_ref_storage<true>> = {<wi::storage_ref> = { > val = 0x7fffffffcfe0, len = 2, precision = 192}, scratch = {64, > 8589921072}}, static is_sign_extended = <optimized out>} > (gdb) p yi > $4 = {<wide_int_ref_storage<true>> = {<wi::storage_ref> = { > val = 0x7fffffffcbc0, len = 1, precision = 192}, scratch = { > 140737488344768, 140737488343008}}, > static is_sign_extended = <optimized out>} > (gdb) > > Somehow PR 65656 miscompiled: > > 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) > > which turned the expression into true and hit > > val[0] = xi.to_uhwi () >> shift; > result.set_len (1); I think we need a better analysis as we use __builtin_constant_p elsewhere as well. Richard. > > -- > H.J.
On Tue, Jan 26, 2016 at 04:54:43PM +0100, Richard Biener wrote: > > Somehow PR 65656 miscompiled: > > > > 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) > > > > which turned the expression into true and hit > > > > val[0] = xi.to_uhwi () >> shift; > > result.set_len (1); > > I think we need a better analysis as we use __builtin_constant_p > elsewhere as well. Yeah, it would be nice to understand the somehow and what exactly is gong on. So, what is xi.precision, is it variable or constant, what value does it have, has __builtin_constant_p returned 1 when it was supposed to return 0? But then there would be still the precision check. What is xi.len and xi.val[0]? Jakub
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; +}