[C++ PATCH] Fix ICE with GNU TLS var in a template (PR c++/66808, PR c++/69000)
Hi! The following testcases ICE because tsubst_decl seems to assume templates of local VAR_DECLs don't have DECL_LANG_SPECIFIC set for them, but for GNU TLS vars they do, yet nothing clears DECL_TEMPLATE_INFO in the DEC_LANG_SPECIFIC copy. The stale DECL_TEMPLATE_INFO then causes mangling to give up as if the var hasn't been instantiated. For the !local_p case tsubst_decl will set DECL_TEMPLATE_INFO to something new. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2015-12-22 Jakub Jelinek PR c++/66808 PR c++/69000 * pt.c (tsubst_decl): For local_p VAR_DECLs, clear DECL_LANG_SPECIFIC (r). * g++.dg/tls/pr66808.C: New test. * g++.dg/tls/pr69000.C: New test. --- gcc/cp/pt.c.jj 2015-12-17 14:49:34.0 +0100 +++ gcc/cp/pt.c 2015-12-21 14:11:24.411320263 +0100 @@ -12183,6 +12183,8 @@ tsubst_decl (tree t, tree args, tsubst_f DECL_TEMPLATE_INSTANTIATED (r) = 0; if (type == error_mark_node) RETURN (error_mark_node); + if (DECL_LANG_SPECIFIC (r) && local_p) + DECL_TEMPLATE_INFO (r) = NULL_TREE; if (TREE_CODE (type) == FUNCTION_TYPE) { /* It may seem that this case cannot occur, since: --- gcc/testsuite/g++.dg/tls/pr69000.C.jj 2015-12-21 14:03:38.362847547 +0100 +++ gcc/testsuite/g++.dg/tls/pr69000.C 2015-12-21 14:04:17.839291295 +0100 @@ -0,0 +1,19 @@ +// PR c++/69000 +// { dg-do compile } +// { dg-require-effective-target tls } + +class A {}; + +template +struct B +{ + static int *& foo () { static __thread int *c = 0; return c; } +}; + +B d; + +void +bar () +{ + d.foo (); +} --- gcc/testsuite/g++.dg/tls/pr66808.C.jj 2015-12-21 14:06:06.791756074 +0100 +++ gcc/testsuite/g++.dg/tls/pr66808.C 2015-12-21 14:06:02.651814409 +0100 @@ -0,0 +1,10 @@ +// PR c++/66808 +// { dg-do compile { target c++11 } } +// { dg-require-effective-target tls } + +template +class A { + int *b = foo (); + int *foo () { static __thread int a; return &a; } +}; +A b; Jakub
Re: [PATCH, libgomp] Rewire OpenACC async
Ping. On 2015/11/24 6:27 PM, Chung-Lin Tang wrote: > Hi, this patch reworks some of the way that asynchronous copyouts are > implemented for OpenACC in libgomp. > > Before this patch, we had a somewhat confusing way of implementing this > by having two refcounts for each mapping: refcount and async_refcount, > which I never got working again after the last wave of async regressions > showed up. > > So this patch implements what I believe to be a simplification: async_refcount > is removed, and instead of trying to queue the async copyouts during unmapping > we actually do that during the plugin event handling. This requires a addition > of the async stream integer as an argument to the register_async_cleanup > plugin hook, but overall I think this should be more elegant than before. > > This patch fixes the libgomp.oacc-c-c++-common/asyncwait-1.c regression. > It also fixed data-[23].c regressions before, but some other recent check-in > happened to already fixed those. > > Tested without regressions, is this okay for trunk? > > Thanks, > Chung-Lin > > 2015-11-24 Chung-Lin Tang > > * oacc-plugin.h (GOMP_PLUGIN_async_unmap_vars): Add int parameter. > * oacc-plugin.c (GOMP_PLUGIN_async_unmap_vars): Add 'int async' > parameter, use to set async stream around call to gomp_unmap_vars, > call gomp_unmap_vars() with 'do_copyfrom' set to true. > * plugin/plugin-nvptx.c (struct ptx_event): Add 'int val' field. > (event_gc): Adjust event handling loop, collect PTX_EVT_ASYNC_CLEANUP > events and call GOMP_PLUGIN_async_unmap_vars() for each of them. > (event_add): Add int parameter, initialize 'val' field when > adding new ptx_event struct. > (nvptx_evec): Adjust event_add() call arguments. > (nvptx_host2dev): Likewise. > (nvptx_dev2host): Likewise. > (nvptx_wait_async): Likewise. > (nvptx_wait_all_async): Likewise. > (GOMP_OFFLOAD_openacc_register_async_cleanup): Add async parameter, > pass to event_add() call. > * oacc-host.c (host_openacc_register_async_cleanup): Add 'int async' > parameter. > * oacc-mem.c (gomp_acc_remove_pointer): Adjust async case to > call openacc.register_async_cleanup_func() hook. > * oacc-parallel.c (GOACC_parallel_keyed): Likewise. > * target.c (gomp_copy_from_async): Delete function. > (gomp_map_vars): Remove async_refcount. > (gomp_unmap_vars): Likewise. > (gomp_load_image_to_device): Likewise. > (omp_target_associate_ptr): Likewise. > * libgomp.h (struct splay_tree_key_s): Remove async_refcount. > (acc_dispatch_t.register_async_cleanup_func): Add int parameter. > (gomp_copy_from_async): Remove. >
Re: RFA: PR 68913: Provide weak version of __fread_chk for PR61886 test
Hi Jeff, PR 68913 notes that the test gcc.dg/lto/pr61886_0.c test fails on targets whose C library does not provide a __fread_chk function. Since the purpose of the test is to show that GCC will correctly discard the invocation of __fread_chk_warn, we do not need to actually link against a real __fread_chk function. A dummy will do. ? Isn'the purpose of this test to verify the function alias resolution code? I think that it is, but that the test does this by requiring that gcc only generates code that calls __fread_chk. Ie it does not generate any code that calls __fread_chk_warn. If gcc did generate code that calls __fread_chk_warn then that function's warning attribute would be triggered and we would get the warning message associated with it - and the test would fail. Since the test only links, it does not execute, there is no need to have working versions of __fread_chk and __fread_chk_warn available. All that is necessary is that a symbol called __fread_chk is available at link time. (A symbol called __fread_chk_warn is not needed as referencing this symbol triggers a warning, which causes the test to fail). So providing a weak definition of __fread_chk should be sufficient for those runtimes which do not provide their own definition. At least that is my theory... Cheers Nick
Re: RFA: Fix ICE compiling gcc.dg/lto/pr55113_0.c for x86/x86_64
Hi Richard, The patch below is a fix for this problem, although I am not sure if it is the correct fix. The patch selects the corresponding SFmode variant of the DFmode vector type when flag_short_doubles is enabled. Maybe a better patch would be to disallow these builtins when using an ABI breaking option ? I think the option should be simply removed... Tempting - but we are in stage 3... My patch at least fixes the ICE for now. Cheers Nick
RFA: PR 68770: Fix use of uninitialised value in secondary_reload
Hi Guys, The patch below fixes PR 68770 - a warning from valgrind about an uninitialised value being used in the default_secondary_reload. The problem turns out to the in copy_costs which creates its own secondary reload information structure, but it does not initialise all of the fields. One field in particular - t_icode - is examined by default_secondary_reload, and it was this that was triggering the valgrind warning. Tested with no regressions on a x86_64-pc-linux-gnu and a powerpc64-le-linux-gnu toolchain. OK to apply ? Cheers Nick gcc/ChangeLog 2015-12-22 Nick Clifton PR target/68770 * ira-costs.c (copy_cost): Initialise the t_code field of the sri structure. Index: ira-costs.c === --- ira-costs.c (revision 231898) +++ ira-costs.c (working copy) @@ -442,6 +442,9 @@ copy it. */ sri.prev_sri = prev_sri; sri.extra_cost = 0; + /* PR 68770: Secondary reload might examine the t_icode field. */ + sri.t_icode = CODE_FOR_nothing; + secondary_class = targetm.secondary_reload (to_p, x, rclass, mode, &sri); if (secondary_class != NO_REGS)
RFA: PR 66655: Use COFF/PE weak symbols
Hi Guys, The patch below is a proposed fix for PR 66655. The issue I believe is not that the ming32 definition of bind_local_p is wrong, but rather that G++ thinks that it cannot make the decl weak even though bind_local_p says that it should. The answer is to define MAKE_DECL_ONE_ONLY using the COFF/PE weak symbol support now available in gas and the linker. Doing this allows the test to pass. OK to apply ? Cheers Nick gcc/ChangeLog 2015-12-22 Nick Clifton PR target/66655 * config/i386/cygming.h (MAKE_DECL_ONE_ONLY): Use weak symbol support, if available. Index: config/i386/cygming.h === --- config/i386/cygming.h (revision 231898) +++ config/i386/cygming.h (working copy) @@ -432,6 +432,10 @@ fputc ('\n', (FILE)); \ } \ while (0) + +/* Make use of the weak support for ONE_ONLY decls. */ +#undef MAKE_DECL_ONE_ONLY +#define MAKE_DECL_ONE_ONLY(DECL) (DECL_WEAK (DECL) = 1) #endif /* HAVE_GAS_WEAK */ /* FIXME: SUPPORTS_WEAK && TARGET_HAVE_NAMED_SECTIONS is true,
Re: [PATCH][combine] Check WORD_REGISTER_OPERATIONS normally rather than through preprocessor
On 17/12/15 14:49, Segher Boessenkool wrote: Hi Kyrill, On Tue, Dec 15, 2015 at 05:07:41PM +, Kyrill Tkachov wrote: As part of the war on conditional compilation here's an #if check on WORD_REGISTER_OPERATIONS that seems to have been missed out. Bootstrapped and tested on arm, aarch64, x86_64. Is it still ok to commit these kinds of conditional compilation conversions? You could say it is a bugfix, a missed case in the conversion ;-) diff --git a/gcc/combine.c b/gcc/combine.c index 8601d8983ce345e2129dd047b3520d98c0582842..0658a6dbc6df6862df662bc7842c13ed06b36b04 100644 --- a/gcc/combine.c +++ b/gcc/combine.c @@ -11488,10 +11488,10 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1) /* Try a few ways of applying the same transformation to both operands. */ while (1) { -#if !WORD_REGISTER_OPERATIONS /* The test below this one won't handle SIGN_EXTENDs on these machines, so check specially. */ - if (code != GTU && code != GEU && code != LTU && code != LEU + if (!WORD_REGISTER_OPERATIONS && code != GTU && code != GEU + && code != LTU && code != LEU Please keep all the code != together, i.e. + if (!WORD_REGISTER_OPERATIONS + && code != GTU && code != GEU && code != LTU && code != LEU Okay with that change. Thanks. Here's what I'll be committing. Kyrill 2015-12-21 Kyrylo Tkachov * combine.c (simplify_comparison): Convert preprocessor check of WORD_REGISTER_OPERATIONS into runtime check. diff --git a/gcc/combine.c b/gcc/combine.c index dc0d4bd52c717b88608d21dbaffe444eeb68bb2d..36ea6df15010247c96a9fcac1649d3d958d64675 100644 --- a/gcc/combine.c +++ b/gcc/combine.c @@ -11436,10 +11436,10 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1) /* Try a few ways of applying the same transformation to both operands. */ while (1) { -#if !WORD_REGISTER_OPERATIONS /* The test below this one won't handle SIGN_EXTENDs on these machines, so check specially. */ - if (code != GTU && code != GEU && code != LTU && code != LEU + if (!WORD_REGISTER_OPERATIONS + && code != GTU && code != GEU && code != LTU && code != LEU && GET_CODE (op0) == ASHIFTRT && GET_CODE (op1) == ASHIFTRT && GET_CODE (XEXP (op0, 0)) == ASHIFT && GET_CODE (XEXP (op1, 0)) == ASHIFT @@ -11459,7 +11459,6 @@ simplify_comparison (enum rtx_code code, rtx *pop0, rtx *pop1) op0 = SUBREG_REG (XEXP (XEXP (op0, 0), 0)); op1 = SUBREG_REG (XEXP (XEXP (op1, 0), 0)); } -#endif /* If both operands are the same constant shift, see if we can ignore the shift. We can if the shift is a rotate or if the bits shifted out of
Re: [RFC][PATCH] Fix broken handling of LABEL_REF in genrecog + genpreds.
On Fri, Dec 18, 2015 at 09:55:54AM +, Richard Sandiford wrote: > Dominik Vogt writes: > > The attached patch fixes the handling of LABEL_REF in genrecog and > > genpreds. > > > > The current code assumes that X can have only a mode than PRED (X, > > MODE) if X is CONST_INT, CONST_DOUBLE or CONST_WIDE_INT, but > > actually that can be also the case for a LABEL_REF with VOIDmode. > > Due to this it is necessary to add "const_int" (or similar) to > > match_code of some predicates handling label_ref but not > > const_int. > > OK, thanks. > > As mentioned in the other thread, I think LABEL_REFs shouldn't have > VOIDmode, so long-term we should be fixing the targets. I agree this > is the correct workaround until that happens though. All right, in the mean time the patch has passed the test suite on x86_64, s390x and s390. See attached version 2 of the patch (added just an additional comment in genpreds.c). Ciao Dominik ^_^ ^_^ -- Dominik Vogt IBM Germany gcc/ChangeLog * config/s390/predicates.md ("larl_operand"): Remove now superfluous const_int and const_double. * genrecog.c (safe_predicate_mode): Return false for VOIDmode LABEL_REFs even if the predicate does not handle const_int, const_double or const_wide_int. * genpreds.c (add_mode_tests): Treat LABEL_REF like CONST_INT. >From 84c51fbb773cb62a24b150063428ba74e42f4146 Mon Sep 17 00:00:00 2001 From: Dominik Vogt Date: Fri, 11 Dec 2015 17:14:25 +0100 Subject: [PATCH] Fix handling of LABEL_REF in safe_predicate_mode. The old code assumes that X can have only a mode than PRED (X, MODE) if X is CONST_INT, CONST_DOUBLE or CONST_WIDE_INT, but actually that can be also the case for LABEL_REF. Due to this it was necessary to add "const_int" (or similar) to match_code of some predicates handling label_ref but not const_int. --- gcc/config/s390/predicates.md | 5 + gcc/genpreds.c| 2 ++ gcc/genrecog.c| 3 ++- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/gcc/config/s390/predicates.md b/gcc/config/s390/predicates.md index 02a1e4e..1211cf01 100644 --- a/gcc/config/s390/predicates.md +++ b/gcc/config/s390/predicates.md @@ -122,10 +122,7 @@ ;; Return true if OP a valid operand for the LARL instruction. (define_predicate "larl_operand" -; Note: Although CONST_INT and CONST_DOUBLE are not handled in this predicate, -; at least one of them needs to appear or otherwise safe_predicate_mode will -; assume that a VOIDmode LABEL_REF is not accepted either (see genrecog.c). - (match_code "label_ref, symbol_ref, const, const_int, const_double") + (match_code "label_ref, symbol_ref, const") { /* Allow labels and local symbols. */ if (GET_CODE (op) == LABEL_REF) diff --git a/gcc/genpreds.c b/gcc/genpreds.c index eac2180..c82113d 100644 --- a/gcc/genpreds.c +++ b/gcc/genpreds.c @@ -320,6 +320,8 @@ add_mode_tests (struct pred_data *p) { case CONST_INT: case CONST_WIDE_INT: + /* Special handling for (VOIDmode) LABEL_REFs. */ + case LABEL_REF: matches_const_scalar_int_p = true; break; diff --git a/gcc/genrecog.c b/gcc/genrecog.c index 599121f..81ea35b 100644 --- a/gcc/genrecog.c +++ b/gcc/genrecog.c @@ -3382,7 +3382,8 @@ safe_predicate_mode (const struct pred_data *pred, machine_mode mode) if (GET_MODE_CLASS (mode) == MODE_INT && (pred->codes[CONST_INT] || pred->codes[CONST_DOUBLE] - || pred->codes[CONST_WIDE_INT])) + || pred->codes[CONST_WIDE_INT] + || pred->codes[LABEL_REF])) return false; return !pred->special && mode != VOIDmode; -- 2.3.0
Re: [Patch testsuite] Skip gcc.dg/ifcvt-4.c for targets on which it may not work
On 21/12/15 19:38, Jeff Law wrote: > On 12/18/2015 02:55 AM, James Greenhalgh wrote: >> This is a multi-part message in MIME format. >> --2.2.0.1.gd394abb.dirty >> Content-Type: text/plain; charset=UTF-8; format=fixed >> Content-Transfer-Encoding: 8bit >> >> >> Hi, >> >> PR68232 is a testsuite failure for targets with very low branch costs. >> As the test is looking for if-conversion, it will fail for any subtarget >> for which the cost of a branch is sufficiently low that if-conversion >> looks >> more expensive. >> >> In the current implementation this will be any subtarget with an >> unpredictable >> branch cost of 0 or 1. I had thought this would be very few targets, >> but >> at least powerpc64le and arm can trigger this for particular tuning >> targets. >> >> This patch skips the test on those targets. >> >> OK? >> >> Thanks, >> James >> >> --- >> 2015-12-17 James Greenhalgh >> >> PR testsuite/68232 >> * gcc.dg/ifcvt-4.c: Skip for arm*-*-* and powerpc64le*-*-*. > OK. > > FWIW, I'd really like to see someone cleanly tweak config-list.mk so > that we can use it to test this kind of stuff. > > In theory what I want to be able to do is build all the listed targets > and run a single test on them so that we can build these skip/xfail > lists much easier. > > I've done it a few times by hand and it seems like it's something we > ought to be able to do much more easily. > > Jeff > The bigger problem here is that branch costs are a property of a specific CPU, not the target architecture. So deciding whether or not we should skip this test (and perhaps others like it) is impossible given that we can't know what the default CPU for the compiler is (and even if we did know, maintaining such a list would be almost impossible). R.
fix VXWORKS_LIBGCC_SPEC not to include -lc_internal for shared rtps
Hello, This is part of a set of changes we have had in-house for a while to improve the support of shared libraries on VxWorks. Today, the port links with libc_internal ("a superset of libgcc.a that we want to use" says comment) for RTPs, static or dynamic. Shared RTPs on VxWorks are linked with the VxWorks libc.so, which also contains everything libgcc related. libc_internal isn't needed in such configurations, and even problematic because it brings in alternate symbol definition. The proposed change fixes this. We have been using it successfully for a while on gcc-4.9 now, and I checked that a mainline compiler configured for powerpc-wrs-vxworks builds after the change. OK to commit ? 2015-12-22 Olivier Hainque * config/vxworks.h (VXWORKS_LIBGCC_SPEC): Don't link shared RTPs with libc_internal. link-shared-rtp.diff Description: Binary data
Re: [patch] libstdc++/59768 Fix std::invoke support for reference_wrappers
On 11/12/15 21:43 +, Jonathan Wakely wrote: Fix std::invoke support for reference_wrappers PR libstdc++/59768 * include/std/functional (_Unwrap, __invfwd): Define. (__invoke_impl): Remove reference_wrapper overloads and use __invfwd. * include/std/type_traits (__result_of_memobj, __result_of_memfun): Add partial specializations for const reference_wrappers and simplify. * testsuite/20_util/bind/ref_neg.cc: Use dg-excess-errors. * testsuite/20_util/function_objects/invoke/59768.cc: New. This change was missing an inline specifier on the new __invfwd function template. Tested ppc64le-linux, committed to trunk. commit a8177a9b95226cc5499d32ecee2034640d881f96 Author: Jonathan Wakely Date: Tue Dec 22 11:20:09 2015 + Add inline to std::__invfwd PR libstdc++/59768 * include/std/functional (__invfwd): Add inline specifier. diff --git a/libstdc++-v3/include/std/functional b/libstdc++-v3/include/std/functional index b994df4..9b853e8 100644 --- a/libstdc++-v3/include/std/functional +++ b/libstdc++-v3/include/std/functional @@ -207,7 +207,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION // Used by __invoke_impl instead of std::forward<_Tp> so that a // reference_wrapper is converted to an lvalue-reference. template -typename _Unwrap<_Tp>::type +inline typename _Unwrap<_Tp>::type __invfwd(typename remove_reference<_Tp>::type& __t) noexcept { return _Unwrap<_Tp>::_S_fwd(__t); }
Re: [rb-general] [PATCH] gcc: read -fdebug-prefix-map OLD from environment (improved reproducibility)
I just found a related patch again that we have in NetBSD's gcc that allows fixing __FILE__ references. See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=47047 No progress since 2010 in getting this included though :( Thomas On Thu, Dec 10, 2015 at 12:36:18PM -0500, Daniel Kahn Gillmor wrote: > Work on the reproducible-builds project [0] has identified that build > paths are one cause of output variation between builds. This > changeset allows users to avoid this variation when building C objects > with debug symbols, while leaving the default behavior unchanged. > > Background > -- > > gcc includes the build path in any generated DWARF debugging symbols, > specifically in DW_AT_comp_dir, but allows the embedded path to be > changed via -fdebug-prefix-map. > > When -fdebug-prefix-map is used with the current build path, it > removes the build path from DW_AT_comp_dir but places it instead in > DW_AT_producer, so the reproducibility problem isn't resolved. > > When building software for binary redistribution, the actual build > path on the build machine is irrelevant, and doesn't need to be > exposed in the debug symbols. > > Resolution > -- > > This patch extends the first argument to -fdebug-prefix-map ("old") to > be able to read from the environment, which allows a packager to avoid > embedded build paths in the debugging symbols with something like: > > export SOURCE_BUILD_DIR="$(pwd)" > gcc -fdebug-prefix-map=\$SOURCE_BUILD_DIR=/usr/src > > Details > --- > > Specifically, if the first character of the "old" argument is a > literal $, then gcc will treat it as an environment variable name, and > use the value of the env var for prefix mapping. > > As a result, DW_AT_producer contains the literal envvar name, > DW_AT_comp_dir contains the transformed build path, and the actual > build path is not at all present in the generated object file. > > This has been tested successfully on amd64 machines, and i see no > reason why it would be platform-specific. > > More discussion of alternate approaches considered and discarded in > the development of this change can be found at [1] for those > interested. > > Feedback welcome! > > [0] https://reproducible-builds.org > [1] > https://lists.alioth.debian.org/pipermail/reproducible-builds/Week-of-Mon-20151130/004051.html > --- > gcc/doc/invoke.texi | 4 +++- > gcc/final.c | 27 +-- > 2 files changed, 28 insertions(+), 3 deletions(-) > > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index 5256031..234432f 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -6440,7 +6440,9 @@ link processing time. Merging is enabled by default. > @item -fdebug-prefix-map=@var{old}=@var{new} > @opindex fdebug-prefix-map > When compiling files in directory @file{@var{old}}, record debugging > -information describing them as in @file{@var{new}} instead. > +information describing them as in @file{@var{new}} instead. If > +@file{@var{old}} starts with a @samp{$}, the corresponding environment > +variable will be dereferenced, and its value will be used instead. > > @item -fno-dwarf2-cfi-asm > @opindex fdwarf2-cfi-asm > diff --git a/gcc/final.c b/gcc/final.c > index 8cb5533..bc43b61 100644 > --- a/gcc/final.c > +++ b/gcc/final.c > @@ -1525,6 +1525,9 @@ add_debug_prefix_map (const char *arg) > { >debug_prefix_map *map; >const char *p; > + char *env; > + const char *old; > + size_t oldlen; > >p = strchr (arg, '='); >if (!p) > @@ -1532,9 +1535,29 @@ add_debug_prefix_map (const char *arg) >error ("invalid argument %qs to -fdebug-prefix-map", arg); >return; > } > + if (*arg == '$') > +{ > + env = xstrndup (arg+1, p - (arg+1)); > + old = getenv(env); > + if (!old) > + { > + warning (0, "environment variable %qs not set in argument to " > +"-fdebug-prefix-map", env); > + free(env); > + return; > + } > + oldlen = strlen(old); > + free(env); > +} > + else > +{ > + old = xstrndup (arg, p - arg); > + oldlen = p - arg; > +} > + >map = XNEW (debug_prefix_map); > - map->old_prefix = xstrndup (arg, p - arg); > - map->old_len = p - arg; > + map->old_prefix = old; > + map->old_len = oldlen; >p++; >map->new_prefix = xstrdup (p); >map->new_len = strlen (p); > -- > 2.6.2 > > ___ > rb-gene...@lists.reproducible-builds.org mailing list > > To unsubscribe or change your options, please visit: > https://lists.reproducible-builds.org/listinfo/rb-general >
Re: [embedded-5-branch][PATCH 0/2] Backporting algorithmic optimization and testcase change
Hi Andre, On Tue, 17 Nov 2015, Andre Vieira wrote: > This series is aimed at backporting algorithmic optimizations and a > change to a test it affects from trunk to the embedded-5-branch. I do not see such a branch documented in https://gcc.gnu.org/svn.html ? Mind adding it there? (If you don't know how to go about it, provide me the text along the lines of what you want to add or provide a patch against the actual web page, and I'll take care.) As to where to put it on this page, would https://gcc.gnu.org/svn.html#distrobranches be appropriate or some other section? Gerald
Fortran release notes (was: [PATCH] v2 ...)
On Thu, 10 Dec 2015, Tobias Burnus wrote: > PS: Talking about the release notes, my feeling is that both the wiki and > the release notes miss some changes, but I have to admit that I am really > out of sync. It currently only lists Submodules at the Wiki, >https://gcc.gnu.org/wiki/GFortran/News#GCC6 > and https://gcc.gnu.org/gcc-6/changes.html has a few other items. (Both > should be synced crosswise.) I would be really good to see all changes land in changes.html in time, since this is what the majority of users -- and press, as I have seen -- consumes. (Why do the Wiki and the formal release notes need to be synced cross-wise? Couldn't you just move things from the Wiki to the release notes to avoid duplication?) Gerald
[wwwdocs] Document how to add a new SSH key
Jan (Beulich) ran into this, and indeed I could not find it documented. So I added it. ;-) Applied. Gerald --- svnwrite.html.orig 2015-04-06 20:15:18.0 +0800 +++ svnwrite.html 2015-12-22 17:07:33.636074381 +0800 @@ -420,6 +420,10 @@ ssh username@gcc.gnu.org email mynewaddr...@example.com +Similarly if you want to add a new SSH key to your account: + +ssh username@gcc.gnu.org append-key < KEYFILE +
C PATCH to warn when accessing members of atomic structures (PR c/69002)
This patch adds a warning (enabled by default) to warn about accessing elements of atomic structures or unions, which is undefined behavior according to the C standard. I didn't make this a translation-time error because it's unclear if it should be so. Bootstrapped/regtested on x86_64-linux and powerpc64le-linux, ok for trunk? 2015-12-22 Marek Polacek PR c/69002 * c-typeck.c (build_component_ref): Warn when acessing elements of atomic structures or unions. * gcc.dg/c11-atomic-1.c: Add dg-warnings. * gcc.dg/c11-atomic-4.c: New test. diff --git gcc/c/c-typeck.c gcc/c/c-typeck.c index a97774f..928fcd5 100644 --- gcc/c/c-typeck.c +++ gcc/c/c-typeck.c @@ -2349,6 +2349,18 @@ build_component_ref (location_t loc, tree datum, tree component) return error_mark_node; } + /* Accessing elements of atomic structures or unions is undefined +behavior (C11 6.5.2.3#5). */ + if (TYPE_ATOMIC (type) && c_inhibit_evaluation_warnings == 0) + { + if (code == RECORD_TYPE) + warning_at (loc, 0, "accessing a member %qE of an atomic " + "structure %qE", component, datum); + else + warning_at (loc, 0, "accessing a member %qE of an atomic " + "union %qE", component, datum); + } + /* Chain the COMPONENT_REFs if necessary down to the FIELD. This might be better solved in future the way the C++ front end does it - by giving the anonymous entities each a diff --git gcc/testsuite/gcc.dg/c11-atomic-1.c gcc/testsuite/gcc.dg/c11-atomic-1.c index c7f9a1e..9702a10 100644 --- gcc/testsuite/gcc.dg/c11-atomic-1.c +++ gcc/testsuite/gcc.dg/c11-atomic-1.c @@ -61,15 +61,15 @@ func (_Atomic volatile long al1) accessing elements of atomic structures and unions is at translation or execution time; presume here that it's at execution time. */ - t1.i = at1.i; - at1.i = t1.i; - atp1->i = t1.i; + t1.i = at1.i; /* { dg-warning "accessing a member .i. of an atomic structure" } */ + at1.i = t1.i; /* { dg-warning "accessing a member .i. of an atomic structure" } */ + atp1->i = t1.i; /* { dg-warning "accessing a member .i. of an atomic structure" } */ au1 = u1; u1 = au1; av1 = v1; v1 = av1; - v1.i = av1.i; - av1.i = v1.i; + v1.i = av1.i; /* { dg-warning "accessing a member .i. of an atomic union" } */ + av1.i = v1.i; /* { dg-warning "accessing a member .i. of an atomic union" } */ /* _Atomic is valid on register variables, even if not particularly useful. */ register _Atomic volatile int ra1 = 1, ra2 = 2; diff --git gcc/testsuite/gcc.dg/c11-atomic-4.c gcc/testsuite/gcc.dg/c11-atomic-4.c index e69de29..81003aa 100644 --- gcc/testsuite/gcc.dg/c11-atomic-4.c +++ gcc/testsuite/gcc.dg/c11-atomic-4.c @@ -0,0 +1,92 @@ +/* PR c/69002 */ +/* Test we diagnose accessing elements of atomic structures or unions, + which is undefined behavior (C11 6.5.2.3#5). */ +/* { dg-do compile } */ +/* { dg-options "-std=c11 -pedantic-errors" } */ + +struct S { int x; }; +union U { int x; }; + +int +fn1 (_Atomic struct S p) +{ + int e = 0 && p.x; + return p.x + e; /* { dg-warning "accessing a member .x. of an atomic structure" } */ +} + +int +fn2 (_Atomic struct S *p) +{ + int e = 1 || p->x; + return p->x + e; /* { dg-warning "accessing a member .x. of an atomic structure" } */ +} + +void +fn3 (_Atomic struct S p, int x) +{ + p.x = x; /* { dg-warning "accessing a member .x. of an atomic structure" } */ +} + +void +fn4 (_Atomic struct S *p, int x) +{ + p->x = x; /* { dg-warning "accessing a member .x. of an atomic structure" } */ +} + +int +fn5 (_Atomic struct S p) +{ + /* This is OK: Members can be safely accessed using a non-atomic + object which is assigned to or from the atomic object. */ + struct S s = p; + return s.x; +} + +int +fn6 (_Atomic struct S *p) +{ + struct S s = *p; + return s.x; +} + +int +fn7 (_Atomic union U p) +{ + int e = 0 && p.x; + return p.x + e; /* { dg-warning "accessing a member .x. of an atomic union" } */ +} + +int +fn8 (_Atomic union U *p) +{ + int e = 1 || p->x; + return p->x + e; /* { dg-warning "accessing a member .x. of an atomic union" } */ +} + +void +fn9 (_Atomic union U p, int x) +{ + p.x = x; /* { dg-warning "accessing a member .x. of an atomic union" } */ +} + +void +fn10 (_Atomic union U *p, int x) +{ + p->x = x; /* { dg-warning "accessing a member .x. of an atomic union" } */ +} + +int +fn11 (_Atomic union U p) +{ + /* This is OK: Members can be safely accessed using a non-atomic + object which is assigned to or from the atomic object. */ + union U s = p; + return s.x; +} + +int +fn12 (_Atomic union U *p) +{ + union U s = *p; + return s.x; +} Marek
Re: C PATCH to warn when accessing members of atomic structures (PR c/69002)
On Tue, 22 Dec 2015, Marek Polacek wrote: > This patch adds a warning (enabled by default) to warn about accessing > elements > of atomic structures or unions, which is undefined behavior according to the C > standard. I didn't make this a translation-time error because it's unclear if > it should be so. > > Bootstrapped/regtested on x86_64-linux and powerpc64le-linux, ok for trunk? OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PING][Patch 3/3][Arm] Add support for IEEE-conformant versions of scalar fmin* and fmax*
On 16/12/15 09:31, Kyrill Tkachov wrote: Hi David, On 16/12/15 08:53, David Sherwood wrote: Hi, Here is the last patch of the fmin/fmax change, which adds the optabs to the arm backend. Tested: arm-none-eabi: no regressions Good to go? David Sherwood. ChangeLog: 2015-12-08 David Sherwood gcc/ * config/arm/iterators.md: New iterators. * config/arm/unspecs.md: New unspecs. * config/arm/neon.md: New pattern. * config/arm/vfp.md: Likewise. Please list the new entities you add in parentheses. For example: * config/arm/iterators.md (VMAXMINFNM): New int iterator. (fmaxmin): New int attribute. (fmaxmin): Likewise. Same for the other files. That way one can grep through the ChangeLogs to find when any particular pattern/iterator/whatever was modified. +;; Auto-vectorized forms for the IEEE-754 fmax()/fmin() functions +(define_insn "3" + [(set (match_operand:VCVTF 0 "s_register_operand" "=w") +(unspec:VCVTF [(match_operand:VCVTF 1 "s_register_operand" "w") + (match_operand:VCVTF 2 "s_register_operand" "w")] + VMAXMINFNM))] + "TARGET_NEON && TARGET_FPU_ARMV8" + ".\t%0, %1, %2" + [(set_attr "type" "neon_fp_minmax_s")] +) I would just say "Vector forms" rather than "Auto-vectorized". In principle we can get vector types through other means besides auto-vectorisation. +;; Scalar forms for the IEEE-754 fmax()/fmin() functions +(define_insn "3" + [(set (match_operand:SDF 0 "s_register_operand" "=") +(unspec:SDF [(match_operand:SDF 1 "s_register_operand" "") + (match_operand:SDF 2 "s_register_operand" "")] + VMAXMINFNM))] + "TARGET_HARD_FLOAT && TARGET_VFP5 " + ".\\t%0, %1, %2" + [(set_attr "type" "f_minmax") + (set_attr "conds" "unconditional")] +) + I notice your new test doesn't test the SF variant of this. Could you please add something to test it? Looks good to me otherwise. Thanks, Kyrill I note strong similarities between this patch and Bilyan Borisov's https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01971.html Both add the same UNSPEC_s, and equivalent VMAXMIN(F?)NM. David's adds and attributes, whereas Bilyan reuses some elements of the existing . AFAICT, the patterns they add are in other ways equivalent (same type, condition, modes, alternatives), albeit in different files and constructed using those different iterators, and David's has the standard names (which IIUC we want, so the autovectorizer finds them) whereas Bilyan adds the intrinsics (which we also want)... --Alan
Re: [AArch32][NEON] Implementing vmaxnmQ_ST and vminnmQ_ST intrinsincs.
On 21/12/15 11:58, Bilyan Borisov wrote: This patch implements the vmaxnmQ_ST and vminnmQ_ST intrinsincs. It also implements the __ARM_FEATURE_NUMERIC_MAXMIN macro, which is defined when __ARM_ARCH >= 8, and which enables the intrinsincs. Tested on arm-none-eabi, armeb-none-eabi, arm-none-linux-gnueabihf. --- gcc/ 2015-XX-XX Bilyan Borisov * config/arm/arm-c.c (arm_cpu_builtins): New macro definition. * config/arm/arm_neon.h (vmaxnm_f32): New intrinsinc. (vmaxnmq_f32): Likewise. (vminnm_f32): Likewise. (vminnmq_f32): Likewise. * config/arm/arm_neon_builtins.def (vmaxnm): New builtin. (vminnm): Likewise. * config/arm/iterators.md (VMAXMINNM): New iterator. (maxmin): Updated iterator. * config/arm/neon.md (neon_v, VCVTF): New pattern. * config/arm/unspecs.md (UNSPEC_VMAXNM): New unspec. (UNSPEC_VMINNM): Likewise. gcc/testsuite/ 2015-XX-XX Bilyan Borisov * gcc.target/arm/simd/vmaxnm_f32_1.c: New. * gcc.target/arm/simd/vmaxnmq_f32_1.c: Likewise. * gcc.target/arm/simd/vminnm_f32_1.c: Likewise. * gcc.target/arm/simd/vminnmq_f32_1.c: Likewise. I note strong similarities between this patch and David Sherwood's https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01560.html Both add the same UNSPEC_s, and equivalent VMAXMIN(F?)NM. David's adds and attributes, whereas Bilyan reuses some elements of the existing . AFAICT, the patterns they add are in other ways equivalent (same type, condition, modes, alternatives), albeit in different files and constructed using those different iterators, and David's has the standard names (which IIUC we want, so the autovectorizer finds them) whereas Bilyan adds the intrinsics (which we also want)... --Alan
[PATCH] Improve fold-const comparison checks (PR c++/67376)
Hi! As the testcase shows, we have some regressions in constexpr folding in GCC 5+. One issue is that fold_binary_loc attempts to swap operands of comparison operators only if one of them is constant, most of the optimizations in there are written for constant in arg1 and therefore ok, but the two checks have both operands non-constant and were just folding a.e + 1 != a.e and not a.e + 1 != a.e. Another issue is that fold_comparison does not optimize this, even when it has code to do so. For ADDR_EXPR it uses get_inner_reference, but for POINTER_PLUS_EXPR if the first operand of it is ADDR_EXPR does not use get_inner_reference, therefore in this case we have base a in one case and base a.e in another case. Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2015-12-22 Jakub Jelinek PR c++/67376 * fold-const.c (size_low_cst): Removed. (fold_comparison): For POINTER_PLUS_EXPR where base is ADDR_EXPR call get_inner_reference and handle INDIRECT_REF base of it. Use offset_int for computation of the bitpos. (fold_binary_loc) : Formatting fixes for X +- Y CMP X and C - X CMP X folding. Add X CMP X +- Y and X CMP C - X folding. * g++.dg/cpp0x/constexpr-67376.C: New test. --- gcc/fold-const.c.jj 2015-12-07 22:04:39.0 +0100 +++ gcc/fold-const.c2015-12-22 14:18:50.099415697 +0100 @@ -8266,20 +8266,6 @@ pointer_may_wrap_p (tree base, tree offs return total.to_uhwi () > (unsigned HOST_WIDE_INT) size; } -/* Return the HOST_WIDE_INT least significant bits of T, a sizetype - kind INTEGER_CST. This makes sure to properly sign-extend the - constant. */ - -static HOST_WIDE_INT -size_low_cst (const_tree t) -{ - HOST_WIDE_INT w = TREE_INT_CST_ELT (t, 0); - int prec = TYPE_PRECISION (TREE_TYPE (t)); - if (prec < HOST_BITS_PER_WIDE_INT) -return sext_hwi (w, prec); - return w; -} - /* Subroutine of fold_binary. This routine performs all of the transformations that are common to the equality/inequality operators (EQ_EXPR and NE_EXPR) and the ordering operators @@ -8408,18 +8394,30 @@ fold_comparison (location_t loc, enum tr STRIP_SIGN_NOPS (base0); if (TREE_CODE (base0) == ADDR_EXPR) { - base0 = TREE_OPERAND (base0, 0); - indirect_base0 = true; + base0 + = get_inner_reference (TREE_OPERAND (base0, 0), + &bitsize, &bitpos0, &offset0, &mode, + &unsignedp, &reversep, &volatilep, + false); + if (TREE_CODE (base0) == INDIRECT_REF) + base0 = TREE_OPERAND (base0, 0); + else + indirect_base0 = true; } - offset0 = TREE_OPERAND (arg0, 1); - if (tree_fits_shwi_p (offset0)) - { - HOST_WIDE_INT off = size_low_cst (offset0); - if ((HOST_WIDE_INT) (((unsigned HOST_WIDE_INT) off) - * BITS_PER_UNIT) - / BITS_PER_UNIT == (HOST_WIDE_INT) off) + if (offset0 == NULL_TREE || integer_zerop (offset0)) + offset0 = TREE_OPERAND (arg0, 1); + else + offset0 = size_binop (PLUS_EXPR, offset0, + TREE_OPERAND (arg0, 1)); + if (TREE_CODE (offset0) == INTEGER_CST) + { + offset_int tem = wi::sext (wi::to_offset (offset0), +TYPE_PRECISION (sizetype)); + tem = wi::lshift (tem, LOG2_BITS_PER_UNIT); + tem += bitpos0; + if (wi::fits_shwi_p (tem)) { - bitpos0 = off * BITS_PER_UNIT; + bitpos0 = tem.to_shwi (); offset0 = NULL_TREE; } } @@ -8443,18 +8441,30 @@ fold_comparison (location_t loc, enum tr STRIP_SIGN_NOPS (base1); if (TREE_CODE (base1) == ADDR_EXPR) { - base1 = TREE_OPERAND (base1, 0); - indirect_base1 = true; + base1 + = get_inner_reference (TREE_OPERAND (base1, 0), + &bitsize, &bitpos1, &offset1, &mode, + &unsignedp, &reversep, &volatilep, + false); + if (TREE_CODE (base1) == INDIRECT_REF) + base1 = TREE_OPERAND (base1, 0); + else + indirect_base1 = true; } - offset1 = TREE_OPERAND (arg1, 1); - if (tree_fits_shwi_p (offset1)) - { - HOST_WIDE_INT off = size_low_cst (offset1); - if ((HOST_WIDE_INT) (((unsigned HOST_WIDE_INT) off) - * BITS_PER_UNIT) - / BITS_PER_UNIT == (HOST_WIDE_INT) off) + if (offset1 == NULL_TREE || integer_zerop (offse
Re: [PING][Patch 3/3][Arm] Add support for IEEE-conformant versions of scalar fmin* and fmax*
On 22/12/15 15:15, Alan Lawrence wrote: On 16/12/15 09:31, Kyrill Tkachov wrote: Hi David, On 16/12/15 08:53, David Sherwood wrote: Hi, Here is the last patch of the fmin/fmax change, which adds the optabs to the arm backend. Tested: arm-none-eabi: no regressions Good to go? David Sherwood. ChangeLog: 2015-12-08 David Sherwood gcc/ * config/arm/iterators.md: New iterators. * config/arm/unspecs.md: New unspecs. * config/arm/neon.md: New pattern. * config/arm/vfp.md: Likewise. Please list the new entities you add in parentheses. For example: * config/arm/iterators.md (VMAXMINFNM): New int iterator. (fmaxmin): New int attribute. (fmaxmin): Likewise. Same for the other files. That way one can grep through the ChangeLogs to find when any particular pattern/iterator/whatever was modified. +;; Auto-vectorized forms for the IEEE-754 fmax()/fmin() functions +(define_insn "3" + [(set (match_operand:VCVTF 0 "s_register_operand" "=w") +(unspec:VCVTF [(match_operand:VCVTF 1 "s_register_operand" "w") + (match_operand:VCVTF 2 "s_register_operand" "w")] + VMAXMINFNM))] + "TARGET_NEON && TARGET_FPU_ARMV8" + ".\t%0, %1, %2" + [(set_attr "type" "neon_fp_minmax_s")] +) I would just say "Vector forms" rather than "Auto-vectorized". In principle we can get vector types through other means besides auto-vectorisation. +;; Scalar forms for the IEEE-754 fmax()/fmin() functions +(define_insn "3" + [(set (match_operand:SDF 0 "s_register_operand" "=") +(unspec:SDF [(match_operand:SDF 1 "s_register_operand" "") + (match_operand:SDF 2 "s_register_operand" "")] + VMAXMINFNM))] + "TARGET_HARD_FLOAT && TARGET_VFP5 " + ".\\t%0, %1, %2" + [(set_attr "type" "f_minmax") + (set_attr "conds" "unconditional")] +) + I notice your new test doesn't test the SF variant of this. Could you please add something to test it? Looks good to me otherwise. Thanks, Kyrill I note strong similarities between this patch and Bilyan Borisov's https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01971.html You're right. Both add the same UNSPEC_s, and equivalent VMAXMIN(F?)NM. David's adds and attributes, whereas Bilyan reuses some elements of the existing . AFAICT, the patterns they add are in other ways equivalent (same type, condition, modes, alternatives), albeit in different files and constructed using those different iterators, and David's has the standard names (which IIUC we want, so the autovectorizer finds them) whereas Bilyan adds the intrinsics (which we also want)... So what I think we want is to take Davids' patch (after my earlier comments are addressed) and rebase Bilyans' patch on top of that to add the intrinsics and ACLE macro (and the tests for these, of course) Thanks, Kyrill --Alan
Re: C PATCH to add __atomic_fetch_* optimization for _Atomics (PR c/68908) (v2)
On Mon, 21 Dec 2015, Marek Polacek wrote: > Uuugh, sorry about that. Fixed in this version and I've added a run-time > test to verify this issue. In addition to that, I've also added a new test > that checks the size of lhs_type -- so that we know we can use some _N variant > of the atomic fetch built-in. > > Bootstrapped/regtested on x86_64-linux and powerpc64le-linux, ok for trunk? OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH, i386] Introduce support for PKU instructions.
Hello Uroš, I (hopefully fixed all of inputs, thanks! Updated patch for i386.md in the bottom, rest patch is the same. Bootstrap in progress. New tests pass. Is it ok for trunk if bootstrap will pass? On 20 Dec 11:56, Uros Bizjak wrote: > > +(define_expand "rdpkru" > > + [(set (match_operand:SI 0 "register_operand") > > + (unspec:SI [(const_int 0)] UNSPEC_PKU)) > > + (set (reg:SI CX_REG) > > + (const_int 0)) > > + (clobber (reg:SI DX_REG))] > > + "TARGET_PKU" > > +{ > > + emit_move_insn (gen_rtx_REG (SImode, CX_REG), CONST0_RTX (SImode)); > > + emit_insn (gen_rdpkru_2 (operands[0])); > > + DONE; > > +}) > > You should use "parallel" to emit insn with several parallel > expressions. So, in the preparation statements, you move const0 to a > pseudo, so the RA will later use correct register. And please leave to > the expander to emit the pattern. > > > +(define_insn "rdpkru_2" > > + [(set (match_operand:SI 0 "register_operand" "=a") > > + (unspec:SI [(const_int 0)] UNSPEC_PKU)) > > + (clobber (reg:SI DX_REG)) > > + (use (reg:SI CX_REG))] > > + "TARGET_PKU" > > + "rdpkru" > > + [(set_attr "type" "other")]) > > Please do not use explicit hard registers. There are appropriate > single-reg constraints available for use. Without seeing the > documentation, I think the above should look like: > > (define_insn "*rdpkru" > [(set (match_operand:SI 0 "register_operand" "=a") >(unspec:SI [(match_operand:SI 1 "register_operand" "c")] UNSPEC_PKU)) >(clobber (rmatch_operand "register_operand "=d")) > "TARGET_PKU" > "rdpkru" > [(set_attr "type" "other")]) > > > +(define_expand "wrpkru" > > + [(unspec_volatile:SI [(match_operand:SI 0 "register_operand")] > > UNSPECV_PKU) > > + (set (reg:SI CX_REG) > > + (const_int 0)) > > + (set (reg:SI DX_REG) > > + (const_int 0))] > > + "TARGET_PKU" > > +{ > > + emit_move_insn (gen_rtx_REG (SImode, CX_REG), CONST0_RTX (SImode)); > > + emit_move_insn (gen_rtx_REG (SImode, DX_REG), CONST0_RTX (SImode)); > > + emit_insn (gen_wrpkru_2 (operands[0])); > > + DONE; > > +}) > > + > > +(define_insn "wrpkru_2" > > + [(unspec_volatile:SI [(match_operand:SI 0 "register_operand" "a")] > > UNSPECV_PKU) > > + (use (reg:SI CX_REG)) > > + (use (reg:SI DX_REG))] > > + "TARGET_PKU" > > + "wrpkru" > > + [(set_attr "type" "other")]) > > > Please move all input operands to the insisde of the unspec, but it > looks that this pattern is missing clobber, as in the above rdpkru > pattern. This isns does not clobber any register. > > Uros. -- Thanks, K diff --git a/gcc/config/i386/i386.md b/gcc/config/i386/i386.md index 49b2216..f427ae3 100644 --- a/gcc/config/i386/i386.md +++ b/gcc/config/i386/i386.md @@ -193,6 +193,9 @@ UNSPEC_BNDCU UNSPEC_BNDCN UNSPEC_MPX_FENCE + + ;; For RDPKRU support + UNSPEC_PKU ]) (define_c_enum "unspecv" [ @@ -268,6 +271,9 @@ ;; For CLZERO support UNSPECV_CLZERO + ;; For WRPKRU support + UNSPECV_PKU + ]) ;; Constants to represent rounding modes in the ROUND instruction @@ -19287,6 +19293,47 @@ [(set_attr "type" "imov") (set_attr "mode" "")]) +(define_expand "rdpkru" + [(set (match_operand:SI 2 "register_operand") (const_int 0)) + (parallel [(set (match_operand:SI 0 "register_operand") + (unspec:SI [(match_dup 2)] UNSPEC_PKU)) + (clobber (match_operand:SI 1 "register_operand"))])] + "TARGET_PKU" +{ + operands[1] = gen_reg_rtx (SImode); + operands[2] = gen_reg_rtx (SImode); +}) + +(define_insn "*rdpkru_2" + [(set (match_operand:SI 0 "register_operand" "=a") + (unspec:SI [(match_operand:SI 2 "register_operand" "c")] UNSPEC_PKU)) + (clobber (match_operand:SI 1 "register_operand" "=d"))] + "TARGET_PKU" + "rdpkru" + [(set_attr "type" "other")]) + +(define_expand "wrpkru" + [(set (match_operand:SI 1 "register_operand") + (const_int 0)) + (set (match_operand:SI 2 "register_operand") + (const_int 0)) + (unspec_volatile:SI [(match_operand:SI 0 "register_operand") + (match_dup 1) + (match_dup 2)] UNSPECV_PKU)] + "TARGET_PKU" +{ + operands[1] = gen_reg_rtx (SImode); + operands[2] = gen_reg_rtx (SImode); +}) + +(define_insn "*wrpkru_2" + [(unspec_volatile:SI [(match_operand:SI 0 "register_operand" "a") + (match_operand:SI 1 "register_operand" "c") + (match_operand:SI 2 "register_operand" "d")] UNSPECV_PKU)] + "TARGET_PKU" + "wrpkru" + [(set_attr "type" "other")]) + (include "mmx.md") (include "sse.md") (include "sync.md")
Re: [PATCH 0/4 v3] Fix PR/63679 when --param sra-max-scalarization-size specified
On 21/12/15 13:13, Alan Lawrence wrote: This is a respin of previous patch series: https://gcc.gnu.org/ml/gcc-patches/2015-10/msg03271.html Minus three of the smaller patches having already been committed; with the updated version of the main patch to SRA; and the patches to DOM reworked to avoid constructing gimple stmt's. IMHO this feels quite invasive for stage 3, however, I note the PR/63679 (ssa-dom-cse-2.c being XFAILed on a bunch of platforms, and currently still FAILing on ARM) is a regression from gcc 4.9.1. So I'd ask maintainer's thoughts as to whether we should take this patch set for gcc 6. I've now tested the series on powerpc64-none-linux-gnu (gcc110) and powerpc64le-none-linux-gnu (gcc112). On powerpc64le, this causes the same guality failures as on AArch64: gcc.dg/guality/pr54970.c -O1 line 15 a[0] == 1 gcc.dg/guality/pr54970.c -O1 line 20 a[0] == 1 gcc.dg/guality/pr54970.c -O1 line 25 a[0] == 1 and the same with other optimization flags. (I note that there are quite a lot of guality failures on both powerpc and also aarch64, which are generally not included in the posts on the gcc-testresults mailing list). The same pr54970 tests still seem to pass on powerpc64 big-endian even with the patches. On both powerpc64 and powerpc64le, the ssa-dom-cse-7.c test also fails, because the constant gets pushed to the constant pool :(, which was not the point of that test (I'd tried to construct it to test normalization of MEM_REFs in DOM prior to the SRA changes). So, adding --param sra-max-scalarization-size-Ospeed=32 fixes this once the SRA patch is in (!), or we could xfail on powerpc64*-*-*-*. --Alan
Re: PATCH: PR target/66232: -fPIC -fno-plt -mx32 fails to generate indirect branch via GOT
On Thu, Dec 17, 2015 at 9:33 PM, H.J. Lu wrote: > On Thu, Dec 17, 2015 at 8:49 AM, H.J. Lu wrote: >> Since Pmode is 64-bit with -maddress-mode=long for x32, indirect call >> via GOT slot doesn't need zero_extend. This patch limits *call_got_x32 >> and *call_value_got_x32 patterns to 32-bit Pmode, adds *call_got_x32_long >> and *call_value_got_x32_long for 64-bit Pmode. >> >> OK for trunk if there is no regression? >> >> >> H.J. >> --- >> gcc/ >> >> PR target/66232 >> * config/i386/i386.md (*call_got_x32): Limited to 32-bit Pmode. >> (*call_value_got_x32): Likewise. >> (*call_got_x32_long): New pattern. >> (call_value_got_x32_long): Likewise. >> > > Here is a different approach without adding new patterns. > Either one works. > gcc/ > > PR target/66232 > * config/i386/constraints.md (Bs): Allow GOT slot for x32 with > 64-bit Pmode. > (Bw): Likewise. > (Bz): Likewise. > * config/i386/predicates.md (call_insn_operand): Likewise. > (sibcall_insn_operand): Likewise. > > gcc/testsuite/ > > PR target/66232 > * gcc.target/i386/pr66232-10.c: New test. > * gcc.target/i386/pr66232-11.c: Likewise. > * gcc.target/i386/pr66232-12.c: Likewise. > * gcc.target/i386/pr66232-13.c: Likewise. This one is OK for mainlne Thanks, Uros.
Re: [PATCH 4/4] Un-XFAIL ssa-dom-cse-2.c for most platforms
On 21/12/15 15:33, Bill Schmidt wrote: Not on a stage1 compiler - check_p8vector_hw_available itself requires being able to run executables - I'll check on gcc112. However, both look like they're really about the host (ability to execute an asm instruction), not the target (/ability for gcc to output such an instruction) Hm, that looks like a pervasive problem for powerpc. There are a number of things that are supposed to be testing effective target but rely on check_p8vector_hw_available, which as you note requires executing an instruction and is really about the host. We need to clean that up; I should probably open a bug. Kind of amazed this has gotten past us for a couple of years. Well, I was about to apologize for making a bogus remark. A really "proper" setup, would be to tell dejagnu to run your execution tests in some kind of emulator/simulator (on your host, perhaps one kind of powerpc) that only/additionally runs instructions for the other, _target_, kind of powerpc...and whatever setup you'd need for all that probably does not live in the GCC repository! For now, just XFAILing for powerpc seems the best alternative for this test. Ok, thanks. --Alan
Re: [PATCH 0/4 v3] Fix PR/63679 when --param sra-max-scalarization-size specified
On Tue, 2015-12-22 at 15:54 +, Alan Lawrence wrote: > On 21/12/15 13:13, Alan Lawrence wrote: > > This is a respin of previous patch series: > > https://gcc.gnu.org/ml/gcc-patches/2015-10/msg03271.html > > Minus three of the smaller patches having already been committed; with the > > updated version of the main patch to SRA; and the patches to DOM reworked > > to avoid constructing gimple stmt's. > > > > IMHO this feels quite invasive for stage 3, however, I note the PR/63679 > > (ssa-dom-cse-2.c being XFAILed on a bunch of platforms, and currently still > > FAILing on ARM) is a regression from gcc 4.9.1. So I'd ask maintainer's > > thoughts > > as to whether we should take this patch set for gcc 6. > > > > > > I've now tested the series on powerpc64-none-linux-gnu (gcc110) and > powerpc64le-none-linux-gnu (gcc112). > > On powerpc64le, this causes the same guality failures as on AArch64: > gcc.dg/guality/pr54970.c -O1 line 15 a[0] == 1 > gcc.dg/guality/pr54970.c -O1 line 20 a[0] == 1 > gcc.dg/guality/pr54970.c -O1 line 25 a[0] == 1 > and the same with other optimization flags. (I note that there are quite a > lot > of guality failures on both powerpc and also aarch64, which are generally not > included in the posts on the gcc-testresults mailing list). That's interesting. I never see these for powerpc64le on my internal build system. I wonder what the difference is? > > The same pr54970 tests still seem to pass on powerpc64 big-endian even with > the > patches. > > On both powerpc64 and powerpc64le, the ssa-dom-cse-7.c test also fails, > because > the constant gets pushed to the constant pool :(, which was not the point of > that test (I'd tried to construct it to test normalization of MEM_REFs in DOM > prior to the SRA changes). So, adding --param > sra-max-scalarization-size-Ospeed=32 fixes this once the SRA patch is in (!), > or > we could xfail on powerpc64*-*-*-*. If this is a democracy, I'd vote for adding the --param to keep the test active, but I won't lie down on the tracks over it. ;) Thanks for including the powerpc64 testing! Bill > > --Alan >
Re: [PATCH 4/4] Un-XFAIL ssa-dom-cse-2.c for most platforms
On Tue, 2015-12-22 at 16:00 +, Alan Lawrence wrote: > On 21/12/15 15:33, Bill Schmidt wrote: > >> > >> Not on a stage1 compiler - check_p8vector_hw_available itself requires > >> being > >> able to run executables - I'll check on gcc112. However, both look like > >> they're > >> really about the host (ability to execute an asm instruction), not the > >> target > >> (/ability for gcc to output such an instruction) > > > > Hm, that looks like a pervasive problem for powerpc. There are a number > > of things that are supposed to be testing effective target but rely on > > check_p8vector_hw_available, which as you note requires executing an > > instruction and is really about the host. We need to clean that up; I > > should probably open a bug. Kind of amazed this has gotten past us for > > a couple of years. > > Well, I was about to apologize for making a bogus remark. A really "proper" > setup, would be to tell dejagnu to run your execution tests in some kind of > emulator/simulator (on your host, perhaps one kind of powerpc) that > only/additionally runs instructions for the other, _target_, kind of > powerpc...and whatever setup you'd need for all that probably does not live > in > the GCC repository! Yeah -- after I wrote that, I looked around some more, and it seems that this is relatively common practice. I agree that it would be pretty tough to set this up properly... > > > For now, just XFAILing for powerpc seems the best alternative for this > > test. > > Ok, thanks. > > > --Alan >
[PATCH] C: fix reported range of invalid unary dereference.
Currently, trunk emits this for a bad unary * in C: bad-dereference.c:10:10: error: invalid type argument of unary ‘*’ (have ‘int’) return *some_f.x; ^ The following patch fixes the reported location to highlight the expression that was attempted to be dereferenced: bad-dereference.c:10:10: error: invalid type argument of unary ‘*’ (have ‘int’) return *some_f.x; ^ Based on another example from: http://clang.llvm.org/diagnostics.html albeit within the "Precision in Wording" example; I didn't change the wording. Successfully bootstrapped®rtested on x86_64-pc-linux-gnu. OK for trunk in stage 3? gcc/c/ChangeLog: * c-parser.c (c_parser_unary_expression): For dereferences, build a combined location before calling build_indirect_ref, so that error reports cover the full range, manually updating the c_expr src_range. gcc/testsuite/ChangeLog: * gcc.dg/bad-dereference.c: New test case. --- gcc/c/c-parser.c | 8 ++-- gcc/testsuite/gcc.dg/bad-dereference.c | 24 2 files changed, 30 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/bad-dereference.c diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index 774354a..a22c3d2 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -6691,8 +6691,12 @@ c_parser_unary_expression (c_parser *parser) op = c_parser_cast_expression (parser, NULL); finish = op.get_finish (); op = convert_lvalue_to_rvalue (exp_loc, op, true, true); - ret.value = build_indirect_ref (op_loc, op.value, RO_UNARY_STAR); - set_c_expr_source_range (&ret, op_loc, finish); + { + location_t combined_loc = make_location (op_loc, op_loc, finish); + ret.value = build_indirect_ref (combined_loc, op.value, RO_UNARY_STAR); + ret.src_range.m_start = op_loc; + ret.src_range.m_finish = finish; + } return ret; case CPP_PLUS: if (!c_dialect_objc () && !in_system_header_at (input_location)) diff --git a/gcc/testsuite/gcc.dg/bad-dereference.c b/gcc/testsuite/gcc.dg/bad-dereference.c new file mode 100644 index 000..5f8188d --- /dev/null +++ b/gcc/testsuite/gcc.dg/bad-dereference.c @@ -0,0 +1,24 @@ +/* { dg-options "-fdiagnostics-show-caret" } */ + +struct foo +{ + int x; +}; + +int test_1 (struct foo some_f) +{ + return *some_f.x; /* { dg-error "invalid type argument of unary ... .have .int.." } */ +/* { dg-begin-multiline-output "" } + return *some_f.x; + ^ + { dg-end-multiline-output "" } */ +} + +int test_2 (struct foo some_f) +{ + return *some_f; /* { dg-error "invalid type argument of unary ... .have .struct foo.." } */ +/* { dg-begin-multiline-output "" } + return *some_f; + ^~~ + { dg-end-multiline-output "" } */ +} -- 1.8.5.3
Re: [PING][Patch 3/3][Arm] Add support for IEEE-conformant versions of scalar fmin* and fmax*
Hi Kyrill, Thanks for the reply, I think this latest patch addresses your comments. I have added a test for the scalar forms of fmax/fmin. Is this ok now? Tested: arm-none-eabi: no regressions Cheers, Dave. ChangeLog: 2015-12-22 David Sherwood gcc/ * config/arm/iterators.md (VMAXMINFNM): New int iterator. (fmaxmin): New int attribute. (fmaxmin_op): Likewise. * config/arm/unspecs.md (UNSPEC_VMAXNM): New unspec. (UNSPEC_VMINNM): Likewise. * config/arm/neon.md (fmaxmin): New pattern. * config/arm/vfp.md (fmaxmin): Likewise. gcc/testsuite * gcc.target/arm/fmaxmin.x: New file used by tests below. * gcc.target/arm/fmaxmin.c: New test. * gcc.target/arm/vect-fmaxmin.c: Likewise. From: Kyrill Tkachov Sent: Wednesday, December 16, 2015 9:31 AM To: David Sherwood; GCC Patches Subject: Re: [PING][Patch 3/3][Arm] Add support for IEEE-conformant versions of scalar fmin* and fmax* Hi David, On 16/12/15 08:53, David Sherwood wrote: > Hi, > > Here is the last patch of the fmin/fmax change, which adds the optabs > to the arm backend. > > Tested: > > arm-none-eabi: no regressions > > Good to go? > David Sherwood. > > ChangeLog: > > 2015-12-08 David Sherwood > > gcc/ > * config/arm/iterators.md: New iterators. > * config/arm/unspecs.md: New unspecs. > * config/arm/neon.md: New pattern. > * config/arm/vfp.md: Likewise. Please list the new entities you add in parentheses. For example: * config/arm/iterators.md (VMAXMINFNM): New int iterator. (fmaxmin): New int attribute. (fmaxmin): Likewise. Same for the other files. That way one can grep through the ChangeLogs to find when any particular pattern/iterator/whatever was modified. +;; Auto-vectorized forms for the IEEE-754 fmax()/fmin() functions +(define_insn "3" + [(set (match_operand:VCVTF 0 "s_register_operand" "=w") + (unspec:VCVTF [(match_operand:VCVTF 1 "s_register_operand" "w") + (match_operand:VCVTF 2 "s_register_operand" "w")] + VMAXMINFNM))] + "TARGET_NEON && TARGET_FPU_ARMV8" + ".\t%0, %1, %2" + [(set_attr "type" "neon_fp_minmax_s")] +) I would just say "Vector forms" rather than "Auto-vectorized". In principle we can get vector types through other means besides auto-vectorisation. +;; Scalar forms for the IEEE-754 fmax()/fmin() functions +(define_insn "3" + [(set (match_operand:SDF 0 "s_register_operand" "=") + (unspec:SDF [(match_operand:SDF 1 "s_register_operand" "") +(match_operand:SDF 2 "s_register_operand" "")] +VMAXMINFNM))] + "TARGET_HARD_FLOAT && TARGET_VFP5 " + ".\\t%0, %1, %2" + [(set_attr "type" "f_minmax") + (set_attr "conds" "unconditional")] +) + I notice your new test doesn't test the SF variant of this. Could you please add something to test it? Looks good to me otherwise. Thanks, Kyrill diff --git a/gcc/config/arm/iterators.md b/gcc/config/arm/iterators.md index c7a6880..6ff346c 100644 --- a/gcc/config/arm/iterators.md +++ b/gcc/config/arm/iterators.md @@ -308,6 +308,8 @@ (define_int_iterator VMAXMINF [UNSPEC_VMAX UNSPEC_VMIN]) +(define_int_iterator VMAXMINFNM [UNSPEC_VMAXNM UNSPEC_VMINNM]) + (define_int_iterator VPADDL [UNSPEC_VPADDL_S UNSPEC_VPADDL_U]) (define_int_iterator VPADAL [UNSPEC_VPADAL_S UNSPEC_VPADAL_U]) @@ -745,6 +747,13 @@ (UNSPEC_VPMIN "min") (UNSPEC_VPMIN_U "min") ]) +(define_int_attr fmaxmin [ + (UNSPEC_VMAXNM "fmax") (UNSPEC_VMINNM "fmin")]) + +(define_int_attr fmaxmin_op [ + (UNSPEC_VMAXNM "vmaxnm") (UNSPEC_VMINNM "vminnm") +]) + (define_int_attr shift_op [ (UNSPEC_VSHL_S "shl") (UNSPEC_VSHL_U "shl") (UNSPEC_VRSHL_S "rshl") (UNSPEC_VRSHL_U "rshl") diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md index 844ef5e..d8cc686 100644 --- a/gcc/config/arm/neon.md +++ b/gcc/config/arm/neon.md @@ -2366,6 +2366,17 @@ [(set_attr "type" "neon_fp_minmax_s")] ) +;; Vector forms for the IEEE-754 fmax()/fmin() functions +(define_insn "3" + [(set (match_operand:VCVTF 0 "s_register_operand" "=w") + (unspec:VCVTF [(match_operand:VCVTF 1 "s_register_operand" "w") + (match_operand:VCVTF 2 "s_register_operand" "w")] + VMAXMINFNM))] + "TARGET_NEON && TARGET_FPU_ARMV8" + ".\t%0, %1, %2" + [(set_attr "type" "neon_fp_minmax_s")] +) + (define_expand "neon_vpadd" [(match_operand:VD 0 "s_register_operand" "=w") (match_operand:VD 1 "s_register_operand" "w") diff --git a/gcc/config/arm/unspecs.md b/gcc/config/arm/unspecs.md index ffe703c..9c633c0 100644 --- a/gcc/config/arm/unspecs.md +++ b/gcc/config/arm/unspecs.md @@ -226,8 +226,10 @@ UNSPEC_VLD4_LANE UNSPEC_VMAX UNSPEC_VMAX_U + UNSPEC_VMAXNM UNSPEC_VMIN UNSPEC_VMIN_U + UNSPEC_VMINNM UNSPEC_VMLA UNSPEC_VMLA_LANE UNSPEC_VMLAL_S diff --git a/gcc/config/arm/vfp.md b/gcc/config/arm/vfp.md index baeac62..3c89fe9
[PATCH] Fix PR66848 by enforcing 16-bit alignment on darwin
The attached patch eliminates the boehm-gc and associated libjava test suite failures on darwin15 due to the recompilation of the system libunwind.dylib with Apple clang 7.0. The new optimizations in Apple Clang 7.0 introduced by... http://llvm.org/viewvc/llvm-project?view=revision&revision=226751 resulted in the movement by the linker of symbols from __bss into __data which exposed a latent bug in the boehm-gc 6.6 release used in FSF gcc. This bug doesn't exist in the more recent boehm-gc 7.2 or later releases. Until the exact change from 6.6 to 7.2 that suppresses this bug is identified or FSF gcc's boehm-gc is rebased on the 7.2 version or later, the simple fix to suppress this issue on darwin is to enforce 16-bit alignment in boehm-gc/include/private/gcconfig.h for that target. The attached change applied to current gcc trunk has been confirmed to bootstrap on x86_64-apple-darwin[13,14,15] without regressions in the boehm-gc and libjava test suites. Note that on darwin15, the build must be installed before testing when the default System Integrity Protection mode is enabled as that prevents DYLD_LIBRARY_PATH from being passed along to helper processes. Okay for gcc trunk and back ports to gcc-5-branch and gcc-4_9-branch? Jack PR66848.patch Description: Binary data
[PATCH] Fix -Wmisleading indentation false-positive for do-while statement
We are emitting a bogus warning for the code do foo (0); while (flagA); ^--- NEXT ^ BODY ^--- GUARD In general I don't think we can get any useful indentation warning out of a do-while statement, so this patch makes it so that we just skip them. Is this OK to commit after testing? gcc/c-family/ChangeLog: * c-indentation.c (should_warn_for_misleading_indentation): Don't warn about do-while statements. gcc/testsuite/ChangeLog: * c-c++-common/Wisleading-indentation.c: Augment test. --- gcc/c-family/c-indentation.c | 6 ++ gcc/testsuite/c-c++-common/Wmisleading-indentation.c | 2 ++ 2 files changed, 8 insertions(+) diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c index 638a9cf..1cbaab7 100644 --- a/gcc/c-family/c-indentation.c +++ b/gcc/c-family/c-indentation.c @@ -202,6 +202,12 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo, if (line_table->seen_line_directive) return false; + /* We can't usefully warn about do-while statements since the bodies of these + statements are always explicitly delimited at both ends, so control flow is + quite obvious. */ + if (guard_tinfo.keyword == RID_DO) +return false; + /* If the token following the body is a close brace or an "else" then while indentation may be sloppy, there is not much ambiguity about control flow, e.g. diff --git a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c index a3f5acd..72c21a0 100644 --- a/gcc/testsuite/c-c++-common/Wmisleading-indentation.c +++ b/gcc/testsuite/c-c++-common/Wmisleading-indentation.c @@ -890,4 +890,6 @@ fn_39 (void) i < 10; i++); foo (i); + + do foo (0); while (flagA); } -- 2.7.0.rc1.98.gacf58d0.dirty
Re: [Patch, fortran] [6 Regression] ICE: in gfc_get_descriptor_dimension, at fortran/trans-array.c:268
Dear Dominique, This will have to wait until after the 28th. For reasons that I cannot determine, I don't seem able to tunnel through to gnu central. Given the imminent arrival of guests for the holiday, I do not feel motivated to investigate further :-) Happy holidays to one and all! Paul On 20 December 2015 at 13:39, Dominique d'Humières wrote: > Dear Paul, >> This is a rather trivial patch... going on 'obvious' in fact. However, >> I must confess to not being entirely sure why the problem is >> occurring. Deferred arrays are emanating from the finalizer that are >> being presented as ARRAY_TYPES rather than descriptors. What ever is >> the reason, the fix is both safe and does what is required. > The patch works as advertised. However the test should be either compile > only, or "dejagnufied". > > Cheers, > > Dominique > -- Outside of a dog, a book is a man's best friend. Inside of a dog it's too dark to read. Groucho Marx
Re: [PATCH] C: fix reported range of invalid unary dereference.
On Tue, Dec 22, 2015 at 12:26:29PM -0500, David Malcolm wrote: > Currently, trunk emits this for a bad unary * in C: > > bad-dereference.c:10:10: error: invalid type argument of unary ‘*’ (have > ‘int’) >return *some_f.x; > ^ > > The following patch fixes the reported location to highlight the > expression that was attempted to be dereferenced: > > bad-dereference.c:10:10: error: invalid type argument of unary ‘*’ (have > ‘int’) >return *some_f.x; > ^ > > Based on another example from: > http://clang.llvm.org/diagnostics.html > albeit within the "Precision in Wording" example; I didn't change the > wording. fwiw gcc's wording seems better to me. > +++ b/gcc/c/c-parser.c > @@ -6691,8 +6691,12 @@ c_parser_unary_expression (c_parser *parser) >op = c_parser_cast_expression (parser, NULL); >finish = op.get_finish (); >op = convert_lvalue_to_rvalue (exp_loc, op, true, true); > - ret.value = build_indirect_ref (op_loc, op.value, RO_UNARY_STAR); > - set_c_expr_source_range (&ret, op_loc, finish); > + { I'd find these braces less confusing if they were for the entire case not just part of it. Trev
Re: [PATCH 0/4 v3] Fix PR/63679 when --param sra-max-scalarization-size specified
On 22/12/15 16:05, Bill Schmidt wrote: On Tue, 2015-12-22 at 15:54 +, Alan Lawrence wrote: On 21/12/15 13:13, Alan Lawrence wrote: This is a respin of previous patch series: https://gcc.gnu.org/ml/gcc-patches/2015-10/msg03271.html Minus three of the smaller patches having already been committed; with the updated version of the main patch to SRA; and the patches to DOM reworked to avoid constructing gimple stmt's. IMHO this feels quite invasive for stage 3, however, I note the PR/63679 (ssa-dom-cse-2.c being XFAILed on a bunch of platforms, and currently still FAILing on ARM) is a regression from gcc 4.9.1. So I'd ask maintainer's thoughts as to whether we should take this patch set for gcc 6. I've now tested the series on powerpc64-none-linux-gnu (gcc110) and powerpc64le-none-linux-gnu (gcc112). On powerpc64le, this causes the same guality failures as on AArch64: gcc.dg/guality/pr54970.c -O1 line 15 a[0] == 1 gcc.dg/guality/pr54970.c -O1 line 20 a[0] == 1 gcc.dg/guality/pr54970.c -O1 line 25 a[0] == 1 and the same with other optimization flags. (I note that there are quite a lot of guality failures on both powerpc and also aarch64, which are generally not included in the posts on the gcc-testresults mailing list). That's interesting. I never see these for powerpc64le on my internal build system. I wonder what the difference is? I've ssh'd into gcc112.fsffrance.org, a power8 powerpc64le system; and configure --with-cpu=power8 --disable-libsanitizer --with-long-double-128 --enable-languages=c,c++,lto. Hmmm...ISTR one variable that can make a big difference is the version (or absence!) of gdb...gcc112 has gdb "Fedora 7.8.2-38.fc21", copyright 2014, and GDB 7.8 looks to have been released at the end of 2014. The same pr54970 tests still seem to pass on powerpc64 big-endian even with the patches. Ach, not quite. In fact those three are failing on powerpc64 bigendian even *without* the patches (at all optimization levels besides -O0 where they pass). This is on gcc110, configure --enable-languages=c,c++,lto --disable-libsanitizer --with-cpu=power7 --with-long-double-128. GDB is Fedora 7.7.1-21.fc20, a bit older, and I tested both unix/-m32 and unix/-m64 (following Bill Seurer's posts on gcc-testresults). I'll email a list of the failures I'm seeing offlist (summary: 186 on gcc112, 148 on gcc110 with -m32, 395 on gcc112 with -m64); however, I suspect gdb version is the difference we are looking for. Which *may* mean that with a more up-to-date GDB, it's possible those failures may not be introduced on ppc64le. (Or similarly AArch64.) Hmmm --Alan
[PATCH, rs6000] Fix PR target/68872, make -mcpu=powerpc64le pass correct assembler option
Currently, -mcpu=powerpc64le correctly sets the TARGET_* flags for an LE compile, meaning it mimics a -mcpu=power8 compile, but it doesn't pass the correct -mpower8/-mpwr8 option to the assembler, so we die with lots of assembler errors on POWER8 instructions. This patch fixes things so we pass the correct assembler option when using -mcpu=powerpc64le. This passes bootstrap/regtesting on powerpc64le-linux. Ok for mainline? Ok for the release branches too after testing? Peter gcc/ PR target/68772 * config/rs6000/rs6000.h (ASM_CPU_SPEC): For -mcpu=powerpc64le, pass %(asm_cpu_power8)/-mpwr8. * config/rs6000/aix53.h: Likewise. * config/rs6000/aix61.h: Likewise. * config/rs6000/aix71.h: Likewise. gcc/testsuite/ PR target/68772 * gcc.target/powerpc/pr68872.c: New test. Index: gcc/config/rs6000/rs6000.h === --- gcc/config/rs6000/rs6000.h (revision 231887) +++ gcc/config/rs6000/rs6000.h (working copy) @@ -128,6 +128,7 @@ %{mcpu=power9: %(asm_cpu_power9)} \ %{mcpu=a2: -ma2} \ %{mcpu=powerpc: -mppc} \ +%{mcpu=powerpc64le: %(asm_cpu_power8)} \ %{mcpu=rs64a: -mppc64} \ %{mcpu=401: -mppc} \ %{mcpu=403: -m403} \ Index: gcc/config/rs6000/aix53.h === --- gcc/config/rs6000/aix53.h (revision 231887) +++ gcc/config/rs6000/aix53.h (working copy) @@ -65,6 +65,7 @@ %{mcpu=power8: -mpwr8} \ %{mcpu=power9: -mpwr9} \ %{mcpu=powerpc: -mppc} \ +%{mcpu=powerpc64le: -mpwr8} \ %{mcpu=rs64a: -mppc} \ %{mcpu=603: -m603} \ %{mcpu=603e: -m603} \ Index: gcc/config/rs6000/aix61.h === --- gcc/config/rs6000/aix61.h (revision 231887) +++ gcc/config/rs6000/aix61.h (working copy) @@ -82,6 +82,7 @@ %{mcpu=power8: -mpwr8} \ %{mcpu=power9: -mpwr9} \ %{mcpu=powerpc: -mppc} \ +%{mcpu=powerpc64le: -mpwr8} \ %{mcpu=rs64a: -mppc} \ %{mcpu=603: -m603} \ %{mcpu=603e: -m603} \ Index: gcc/config/rs6000/aix71.h === --- gcc/config/rs6000/aix71.h (revision 231887) +++ gcc/config/rs6000/aix71.h (working copy) @@ -81,6 +81,7 @@ %{mcpu=power7: -mpwr7} \ %{mcpu=power8: -mpwr8} \ %{mcpu=powerpc: -mppc} \ +%{mcpu=powerpc64le: -mpwr8} \ %{mcpu=rs64a: -mppc} \ %{mcpu=603: -m603} \ %{mcpu=603e: -m603} \ Index: gcc/testsuite/gcc.target/powerpc/pr68872.c === --- gcc/testsuite/gcc.target/powerpc/pr68872.c (revision 0) +++ gcc/testsuite/gcc.target/powerpc/pr68872.c (working copy) @@ -0,0 +1,14 @@ +/* PR target/68872 */ +/* { dg-do assemble { target { powerpc64le-*-* } } } */ +/* { dg-options "-mcpu=powerpc64le" } */ + +/* Verify that -mcpu=powerpc64le passes -mpower8/-mpwr8 to the assembler. */ + +long +bar (unsigned char *ptr, unsigned char val) +{ + long ret; + asm volatile ("stbcx. %0,0,%1" :: "r" (val), "r" (ptr)); + asm volatile ("mfcr %0,8" : "=r" (ret) ::); + return ret; +}
Re: [PATCH, rs6000] Fix PR target/68872, make -mcpu=powerpc64le pass correct assembler option
On Tue, Dec 22, 2015 at 12:13 PM, Peter Bergner wrote: > Currently, -mcpu=powerpc64le correctly sets the TARGET_* flags for an > LE compile, meaning it mimics a -mcpu=power8 compile, but it doesn't > pass the correct -mpower8/-mpwr8 option to the assembler, so we die > with lots of assembler errors on POWER8 instructions. This patch > fixes things so we pass the correct assembler option when using > -mcpu=powerpc64le. > > This passes bootstrap/regtesting on powerpc64le-linux. Ok for mainline? > Ok for the release branches too after testing? > > Peter > > > gcc/ > PR target/68772 > * config/rs6000/rs6000.h (ASM_CPU_SPEC): For -mcpu=powerpc64le, > pass %(asm_cpu_power8)/-mpwr8. > * config/rs6000/aix53.h: Likewise. > * config/rs6000/aix61.h: Likewise. > * config/rs6000/aix71.h: Likewise. > > gcc/testsuite/ > PR target/68772 > * gcc.target/powerpc/pr68872.c: New test. AIX does not support PPC64LE, so the aix53.h, aix61.h and aix71.h changes should not be applied. Ultimately rs6000_file_start ".machine" directive support should be strengthened to handle -mcpu=. Okay with just the rs6000.h change and new testcase. Thanks, David
Re: [Patch testsuite] Skip gcc.dg/ifcvt-4.c for targets on which it may not work
On 12/22/2015 04:24 AM, Richard Earnshaw (lists) wrote: The bigger problem here is that branch costs are a property of a specific CPU, not the target architecture. So deciding whether or not we should skip this test (and perhaps others like it) is impossible given that we can't know what the default CPU for the compiler is (and even if we did know, maintaining such a list would be almost impossible). True, but you still get a reasonable set of targets where the test passes vs fails -- not all the targets have as many variants as ARM :-) I'd certainly agree that covering all the subtargets and then keeping those lists accurate sounds like a lot of make-work. jeff
[patch] Fix dynamic linker spec for FreeBSD powerpc64
Hi all, The attached patch fixes a problem you get if you build dynamic binaries for a 32-bit powerpc target on a 64-bit powerpc host. At the time I did this port I didn't fully understand all the scenarios you might run into. The issue is this, on all FreeBSD archs the interpreter is ld-elf.so.1. On powerpc64 we have also an ld-elf32.so.1 for 32-bit binaries. In case we run a 32-bit binary on a 64-bit host the RTLD reroutes the call to this ld-elf32.so.1. Up to now this rerouting didn't happen since gcc used the ld-elf32.so.1 instead. The binary runs fine on the 64-bit host since the RTLD finds a ld-elf32.so.1. But now when I take this binary to a 32-bit host I can't run it since the RTLD does not find the ld-elf32.so.1. The patch fixes this and simplifies the LINK_OS_FREEBSD_SPEC_DEF for FreeBSD powerpc64. If there are no major objections I'm going to apply this patch to trunk, gcc-5 and gcc-49 branch in the next days. It is FreeBSD powerpc64 only and it is a bug. Thanks, Andreas 2015-12-22 Andreas Tobler * config/rs6000/freebsd64.h: Delete FREEBSD_DYNAMIC_LINKER32/64 defines. Use FBSD_DYNAMIC_LINKER instead. Rename and simplify LINK_OS_FREEBSD_SPEC_DEF32/64 to LINK_OS_FREEBSD_SPEC_DEF. Index: freebsd64.h === --- freebsd64.h (revision 231662) +++ freebsd64.h (working copy) @@ -167,10 +167,7 @@ { "link_os_freebsd_spec32", LINK_OS_FREEBSD_SPEC32 }, \ { "link_os_freebsd_spec64", LINK_OS_FREEBSD_SPEC64 }, -#define FREEBSD_DYNAMIC_LINKER32 "/libexec/ld-elf32.so.1" -#define FREEBSD_DYNAMIC_LINKER64 "/libexec/ld-elf.so.1" - -#define LINK_OS_FREEBSD_SPEC_DEF32 "\ +#define LINK_OS_FREEBSD_SPEC_DEF "\ %{p:%nconsider using `-pg' instead of `-p' with gprof(1)} \ %{v:-V} \ %{assert*} %{R*} %{rpath*} %{defsym*} \ @@ -178,25 +175,13 @@ %{!shared: \ %{!static: \ %{rdynamic: -export-dynamic} \ - %{!dynamic-linker:-dynamic-linker " FREEBSD_DYNAMIC_LINKER32 "}} \ + %{!dynamic-linker:-dynamic-linker " FBSD_DYNAMIC_LINKER "}} \ %{static:-Bstatic}} \ %{symbolic:-Bsymbolic}" -#define LINK_OS_FREEBSD_SPEC_DEF64 "\ - %{p:%nconsider using `-pg' instead of `-p' with gprof(1)} \ - %{v:-V} \ - %{assert*} %{R*} %{rpath*} %{defsym*} \ - %{shared:-Bshareable %{h*} %{soname*}} \ - %{!shared: \ -%{!static: \ - %{rdynamic: -export-dynamic} \ - %{!dynamic-linker:-dynamic-linker " FREEBSD_DYNAMIC_LINKER64 "}} \ -%{static:-Bstatic}} \ - %{symbolic:-Bsymbolic}" - -#define LINK_OS_FREEBSD_SPEC32 "-melf32ppc_fbsd " LINK_OS_FREEBSD_SPEC_DEF32 +#define LINK_OS_FREEBSD_SPEC32 "-melf32ppc_fbsd " LINK_OS_FREEBSD_SPEC_DEF -#define LINK_OS_FREEBSD_SPEC64 "-melf64ppc_fbsd " LINK_OS_FREEBSD_SPEC_DEF64 +#define LINK_OS_FREEBSD_SPEC64 "-melf64ppc_fbsd " LINK_OS_FREEBSD_SPEC_DEF #undef MULTILIB_DEFAULTS #define MULTILIB_DEFAULTS { "m64" }
Re: [PATCH, rs6000] Fix PR target/68872, make -mcpu=powerpc64le pass correct assembler option
On Tue, 2015-12-22 at 13:36 -0500, David Edelsohn wrote: > On Tue, Dec 22, 2015 at 12:13 PM, Peter Bergner wrote: > AIX does not support PPC64LE, so the aix53.h, aix61.h and aix71.h > changes should not be applied. Heh, I should know better. Thanks for catching that. > Ultimately rs6000_file_start ".machine" directive support should be > strengthened to handle -mcpu=. I'm not sure there is anything to do there. The code that emits the .machine assembler directive uses rs6000_isa_flags which should be set correctly, regardless of whether we use -mcpu= with power8 or powerpc64le. ...although, the code as is never outputs a .machine on LE, since the code that guards the emitting of .machine doesn't ever seem to be true on LE. At least I can't seem to get a .machine generated with my LE build. > Okay with just the rs6000.h change and new testcase. Ok, committed to trunk as revision 231905. Thanks. Peter
Re: [PATCH, rs6000] Fix PR target/68872, make -mcpu=powerpc64le pass correct assembler option
On Tue, Dec 22, 2015 at 2:30 PM, Peter Bergner wrote: > On Tue, 2015-12-22 at 13:36 -0500, David Edelsohn wrote: >> On Tue, Dec 22, 2015 at 12:13 PM, Peter Bergner wrote: >> Ultimately rs6000_file_start ".machine" directive support should be >> strengthened to handle -mcpu=. > > I'm not sure there is anything to do there. The code that emits the > .machine assembler directive uses rs6000_isa_flags which should be > set correctly, regardless of whether we use -mcpu= with power8 or > powerpc64le. ...although, the code as is never outputs a .machine > on LE, since the code that guards the emitting of .machine doesn't > ever seem to be true on LE. At least I can't seem to get a .machine > generated with my LE build. Currently, the .machine directive only is emitted if -mcpu= is not used. We should avoid -mcpu= setting assembler command line options and instead switch to use .machine. The .machine path needs to be tested more thoroughly in the presence of -mcpu options. Thanks, David
[patch] ARM FreeBSD fix bootstrap
Hi all, the commit for PR68617 broke boostrap on armv6*-*-freebsd*. We still have unaligned_access = 0 on armv6 here on FreeBSD. The commit from the above PR overrides my SUBTARGET_OVERRIDE_OPTIONS I called in arm_option_override. And it sets the unaligned_access to 1. The attached patch fixes this, bootstrap ongoing but passed the breaking stage where genmddeps bus errored. Is this patch ok for trunk once bootstrap completes? TIA, Andreas 2015-12-22 Andreas Tobler * config/arm/freebsd.h (SUBTARGET_OVERRIDE_OPTIONS): Adjust to check unaligned_access on the gcc_options set. * config/arm/arm.c (arm_option_override): Move SUBTARGET_OVERRIDE_OPTIONS from here to (arm_option_override_internal). Index: config/arm/freebsd.h === --- config/arm/freebsd.h(revision 231903) +++ config/arm/freebsd.h(working copy) @@ -122,8 +122,8 @@ #define SUBTARGET_OVERRIDE_OPTIONS \ do { \ -if (unaligned_access) \ - unaligned_access = 0; \ +if (opts->x_unaligned_access) \ + opts->x_unaligned_access = 0; \ } while (0) #undef MAX_SYNC_LIBFUNC_SIZE Index: config/arm/arm.c === --- config/arm/arm.c(revision 231903) +++ config/arm/arm.c(working copy) @@ -2954,6 +2954,10 @@ /* Thumb2 inline assembly code should always use unified syntax. This will apply to ARM and Thumb1 eventually. */ opts->x_inline_asm_unified = TARGET_THUMB2_P (opts->x_target_flags); + +#ifdef SUBTARGET_OVERRIDE_OPTIONS + SUBTARGET_OVERRIDE_OPTIONS; +#endif } /* Fix up any incompatible options that the user has specified. */ @@ -2976,10 +2980,6 @@ if (global_options_set.x_arm_tune_option) arm_selected_tune = &all_cores[(int) arm_tune_option]; -#ifdef SUBTARGET_OVERRIDE_OPTIONS - SUBTARGET_OVERRIDE_OPTIONS; -#endif - if (arm_selected_arch) { if (arm_selected_cpu)
PR ipa/67811
The problem in this PR is that when we duplicate blocks, we fail to duplicate eh regions. Most of the places that we want to do block duplication, that's fine. We've already identified that we're operating on a SESE region, or suchlike, and so EH doesn't apply -- if EH were present it wouldn't be SESE and we'd not attempt the optimization. Rather than attempt to write a bunch of new code in order to make this work, I'd prefer to duplicate code early, when it's still just medium-level GENERIC nodes, and then let the rather better tested code remove unreachable code and EH regions when necessary. Tested on x86_64 and committed. r~ * gimple.h (struct gtransaction): Add label_norm, label_uninst; replace label with label_over. (gimple_build_transaction): Remove label parameter. (gimple_transaction_label_norm): New. (gimple_transaction_label_uninst): New. (gimple_transaction_label_over): Rename from gimple_transaction_label. (gimple_transaction_label_norm_ptr): New. (gimple_transaction_label_uninst_ptr): New. (gimple_transaction_label_over_ptr): Rename from gimple_transaction_label_ptr. (gimple_transaction_set_label_norm): New. (gimple_transaction_set_label_uninst): New. (gimple_transaction_set_label_over): Rename from gimple_transaction_set_label. * gimple-pretty-print.c (dump_gimple_transaction): Update. * gimple-streamer-in.c (input_gimple_stmt) [GIMPLE_TRANSACTION]: Same. * gimple-streamer-out.c (output_gimple_stmt) [GIMPLE_TRANSACTION]: Same. * gimple-walk.c (walk_gimple_op) [GIMPLE_TRANSACTION]: Same. * tree-cfg.c (make_edges_bb) [GIMPLE_TRANSACTION]: Same. (cleanup_dead_labels) [GIMPLE_TRANSACTION]: Same. (verify_gimple_transaction): Same. (gimple_redirect_edge_and_branch) [GIMPLE_TRANSACTION]: Same. * tree-inline.c (remap_gimple_stmt) [GIMPLE_TRANSACTION]: Same. * gimple.c (gimple_build_transaction): Remove label parameter; initialize all three label memebers. * gimplify.c (gimplify_transaction): Update call to gimple_build_transaction. * trans-mem.c (make_tm_uninst): New. (lower_transaction): Create uninstrumented code path here... (ipa_tm_scan_calls_transaction): ... not here. (ipa_uninstrument_transaction): Remove. testsuite/ * g++.dg/tm/noexcept-1.C: Update expected must_not_throw count. * g++.dg/tm/noexcept-4.C: Likewise. * g++.dg/tm/noexcept-5.C: Likewise. * g++.dg/tm/pr67811.C: New. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@231907 138bc75d-0d04-0410-961f-82ee72b054a4 --- gcc/ChangeLog| 35 +++ gcc/gimple-pretty-print.c| 48 +-- gcc/gimple-streamer-in.c | 8 ++- gcc/gimple-streamer-out.c| 8 ++- gcc/gimple-walk.c| 21 +-- gcc/gimple.c | 6 +- gcc/gimple.h | 59 ++ gcc/gimplify.c | 2 +- gcc/testsuite/ChangeLog | 7 +++ gcc/testsuite/g++.dg/tm/noexcept-1.C | 2 +- gcc/testsuite/g++.dg/tm/noexcept-4.C | 2 +- gcc/testsuite/g++.dg/tm/noexcept-5.C | 2 +- gcc/testsuite/g++.dg/tm/pr67811.C| 11 gcc/trans-mem.c | 115 ++- gcc/tree-cfg.c | 70 - gcc/tree-inline.c| 14 +++-- 16 files changed, 288 insertions(+), 122 deletions(-) create mode 100644 gcc/testsuite/g++.dg/tm/pr67811.C diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 11f53c7..a9e73f4 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,38 @@ +2015-12-22 Richard Henderson + + PR ipa/67811 + * gimple.h (struct gtransaction): Add label_norm, label_uninst; + replace label with label_over. + (gimple_build_transaction): Remove label parameter. + (gimple_transaction_label_norm): New. + (gimple_transaction_label_uninst): New. + (gimple_transaction_label_over): Rename from gimple_transaction_label. + (gimple_transaction_label_norm_ptr): New. + (gimple_transaction_label_uninst_ptr): New. + (gimple_transaction_label_over_ptr): Rename from + gimple_transaction_label_ptr. + (gimple_transaction_set_label_norm): New. + (gimple_transaction_set_label_uninst): New. + (gimple_transaction_set_label_over): Rename from + gimple_transaction_set_label. + * gimple-pretty-print.c (dump_gimple_transaction): Update. + * gimple-streamer-in.c (input_gimple_stmt) [GIMPLE_TRANSACTION]: Same. + * gimple-streamer-out.c (output_gimple_stmt) [GIMPLE_TRANSACTION]: Same. + * gimple-walk.c (walk_gimple_op) [GIMPLE_TRANSACTION]: Same. + * tree-cfg.c (make_edges_bb) [GIMPLE_TRANSACTION]: Same. + (cleanup_dead_labels) [GIMPLE_TRANSACTION]: Same. + (verify_gimple_transaction): Same. + (gimple_redirect_edge_and_branch) [GIMPLE_TRANSACTION]: Same. + * tree-inline.c (remap_gimple_stmt) [GIMPLE_TRANSACTION]: Same. + * gimple.c (gimple_build_transaction): Remove label parameter; + initialize all three label memebers. + * gimplify.c (gimplify_transaction): Update call + to gimple_build_transa
[trans-mem] Give EH_ELSE access to __builtin_eh_pointer
The must-not-throw wrapper for protect_cleanup_actions gets in the way of being able to access __builtin_eh_pointer without confusion as the identit of the exception to which we are referring (b_eh_p has no usable argument up to this point). Since EH_ELSE never comes from user derived code, let's drop the c++ specific wrapping. This fixes a problem wherein _ITM_commitTransactionEH was always receiving a NULL pointer for the current exception. We'd been working around that in the library, but that's somewhat silly... Tested on x86_64 and committed. r~ * tree-eh.c (honor_protect_cleanup_actions): Do not wrap eh_else in a must-not-throw; set ehp_region for it too. git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@231908 138bc75d-0d04-0410-961f-82ee72b054a4 --- gcc/ChangeLog | 3 ++ gcc/tree-eh.c | 97 ++- 2 files changed, 52 insertions(+), 48 deletions(-) diff --git a/gcc/ChangeLog b/gcc/ChangeLog index a9e73f4..473119a 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -33,6 +33,9 @@ (ipa_tm_scan_calls_transaction): ... not here. (ipa_uninstrument_transaction): Remove. + * tree-eh.c (honor_protect_cleanup_actions): Do not wrap eh_else + in a must-not-throw; set ehp_region for it too. + 2015-12-22 Peter Bergner PR target/68772 diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c index 9f68f31..5552fc1 100644 --- a/gcc/tree-eh.c +++ b/gcc/tree-eh.c @@ -986,64 +986,65 @@ honor_protect_cleanup_actions (struct leh_state *outer_state, struct leh_state *this_state, struct leh_tf_state *tf) { - tree protect_cleanup_actions; - gimple_stmt_iterator gsi; - bool finally_may_fallthru; - gimple_seq finally; - gimple *x; - geh_mnt *eh_mnt; - gtry *try_stmt; - geh_else *eh_else; - - /* First check for nothing to do. */ - if (lang_hooks.eh_protect_cleanup_actions == NULL) -return; - protect_cleanup_actions = lang_hooks.eh_protect_cleanup_actions (); - if (protect_cleanup_actions == NULL) -return; - - finally = gimple_try_cleanup (tf->top_p); - eh_else = get_eh_else (finally); + gimple_seq finally = gimple_try_cleanup (tf->top_p); - /* Duplicate the FINALLY block. Only need to do this for try-finally, - and not for cleanups. If we've got an EH_ELSE, extract it now. */ - if (eh_else) + /* EH_ELSE doesn't come from user code; only compiler generated stuff. + It does need to be handled here, so as to separate the (different) + EH path from the normal path. But we should not attempt to wrap + it with a must-not-throw node (which indeed gets in the way). */ + if (geh_else *eh_else = get_eh_else (finally)) { - finally = gimple_eh_else_e_body (eh_else); gimple_try_set_cleanup (tf->top_p, gimple_eh_else_n_body (eh_else)); + finally = gimple_eh_else_e_body (eh_else); + + /* Let the ELSE see the exception that's being processed. */ + eh_region save_ehp = this_state->ehp_region; + this_state->ehp_region = this_state->cur_region; + lower_eh_constructs_1 (this_state, &finally); + this_state->ehp_region = save_ehp; } - else if (this_state) -finally = lower_try_finally_dup_block (finally, outer_state, - gimple_location (tf->try_finally_expr)); - finally_may_fallthru = gimple_seq_may_fallthru (finally); - - /* If this cleanup consists of a TRY_CATCH_EXPR with TRY_CATCH_IS_CLEANUP - set, the handler of the TRY_CATCH_EXPR is another cleanup which ought - to be in an enclosing scope, but needs to be implemented at this level - to avoid a nesting violation (see wrap_temporary_cleanups in - cp/decl.c). Since it's logically at an outer level, we should call - terminate before we get to it, so strip it away before adding the - MUST_NOT_THROW filter. */ - gsi = gsi_start (finally); - x = gsi_stmt (gsi); - if (gimple_code (x) == GIMPLE_TRY - && gimple_try_kind (x) == GIMPLE_TRY_CATCH - && gimple_try_catch_is_cleanup (x)) + else { - gsi_insert_seq_before (&gsi, gimple_try_eval (x), GSI_SAME_STMT); - gsi_remove (&gsi, false); -} + /* First check for nothing to do. */ + if (lang_hooks.eh_protect_cleanup_actions == NULL) + return; + tree actions = lang_hooks.eh_protect_cleanup_actions (); + if (actions == NULL) + return; + + if (this_state) + finally = lower_try_finally_dup_block (finally, outer_state, + gimple_location (tf->try_finally_expr)); + + /* If this cleanup consists of a TRY_CATCH_EXPR with TRY_CATCH_IS_CLEANUP + set, the handler of the TRY_CATCH_EXPR is another cleanup which ought + to be in an enclosing scope, but needs to be implemented at this level + to avoid a nesting violation (see wrap_temporary_cleanups in + cp/decl.c). Since it's logically at an outer level, we should call + terminate before we get to it, so strip it away before adding the + MUST_NOT_THROW filter. */ + gimple_stmt_iterator gsi = gsi_start (finally); + gimple *x
Re: [PATCH] Improve fold-const comparison checks (PR c++/67376)
On December 22, 2015 4:24:40 PM GMT+01:00, Jakub Jelinek wrote: >Hi! > >As the testcase shows, we have some regressions in constexpr folding in >GCC >5+. >One issue is that fold_binary_loc attempts to swap operands of >comparison >operators only if one of them is constant, most of the optimizations in >there are written for constant in arg1 and therefore ok, but the two >checks >have both operands non-constant and were just folding a.e + 1 != a.e >and >not a.e + 1 != a.e. >Another issue is that fold_comparison does not optimize this, even when >it >has code to do so. For ADDR_EXPR it uses get_inner_reference, but for >POINTER_PLUS_EXPR if the first operand of it is ADDR_EXPR does not use >get_inner_reference, therefore in this case we have base a in one case >and >base a.e in another case. > >Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok >for >trunk? OK. Thanks, Richard. >2015-12-22 Jakub Jelinek > > PR c++/67376 > * fold-const.c (size_low_cst): Removed. > (fold_comparison): For POINTER_PLUS_EXPR where base is ADDR_EXPR > call get_inner_reference and handle INDIRECT_REF base of it. Use > offset_int for computation of the bitpos. > (fold_binary_loc) : Formatting > fixes for X +- Y CMP X and C - X CMP X folding. Add X CMP X +- Y > and X CMP C - X folding. > > * g++.dg/cpp0x/constexpr-67376.C: New test. > >--- gcc/fold-const.c.jj2015-12-07 22:04:39.0 +0100 >+++ gcc/fold-const.c 2015-12-22 14:18:50.099415697 +0100 >@@ -8266,20 +8266,6 @@ pointer_may_wrap_p (tree base, tree offs > return total.to_uhwi () > (unsigned HOST_WIDE_INT) size; > } > >-/* Return the HOST_WIDE_INT least significant bits of T, a sizetype >- kind INTEGER_CST. This makes sure to properly sign-extend the >- constant. */ >- >-static HOST_WIDE_INT >-size_low_cst (const_tree t) >-{ >- HOST_WIDE_INT w = TREE_INT_CST_ELT (t, 0); >- int prec = TYPE_PRECISION (TREE_TYPE (t)); >- if (prec < HOST_BITS_PER_WIDE_INT) >-return sext_hwi (w, prec); >- return w; >-} >- > /* Subroutine of fold_binary. This routine performs all of the >transformations that are common to the equality/inequality >operators (EQ_EXPR and NE_EXPR) and the ordering operators >@@ -8408,18 +8394,30 @@ fold_comparison (location_t loc, enum tr > STRIP_SIGN_NOPS (base0); > if (TREE_CODE (base0) == ADDR_EXPR) > { >-base0 = TREE_OPERAND (base0, 0); >-indirect_base0 = true; >+base0 >+ = get_inner_reference (TREE_OPERAND (base0, 0), >+ &bitsize, &bitpos0, &offset0, &mode, >+ &unsignedp, &reversep, &volatilep, >+ false); >+if (TREE_CODE (base0) == INDIRECT_REF) >+ base0 = TREE_OPERAND (base0, 0); >+else >+ indirect_base0 = true; > } >-offset0 = TREE_OPERAND (arg0, 1); >-if (tree_fits_shwi_p (offset0)) >- { >-HOST_WIDE_INT off = size_low_cst (offset0); >-if ((HOST_WIDE_INT) (((unsigned HOST_WIDE_INT) off) >- * BITS_PER_UNIT) >-/ BITS_PER_UNIT == (HOST_WIDE_INT) off) >+if (offset0 == NULL_TREE || integer_zerop (offset0)) >+ offset0 = TREE_OPERAND (arg0, 1); >+else >+ offset0 = size_binop (PLUS_EXPR, offset0, >+TREE_OPERAND (arg0, 1)); >+if (TREE_CODE (offset0) == INTEGER_CST) >+ { >+offset_int tem = wi::sext (wi::to_offset (offset0), >+ TYPE_PRECISION (sizetype)); >+tem = wi::lshift (tem, LOG2_BITS_PER_UNIT); >+tem += bitpos0; >+if (wi::fits_shwi_p (tem)) > { >-bitpos0 = off * BITS_PER_UNIT; >+bitpos0 = tem.to_shwi (); > offset0 = NULL_TREE; > } > } >@@ -8443,18 +8441,30 @@ fold_comparison (location_t loc, enum tr > STRIP_SIGN_NOPS (base1); > if (TREE_CODE (base1) == ADDR_EXPR) > { >-base1 = TREE_OPERAND (base1, 0); >-indirect_base1 = true; >+base1 >+ = get_inner_reference (TREE_OPERAND (base1, 0), >+ &bitsize, &bitpos1, &offset1, &mode, >+ &unsignedp, &reversep, &volatilep, >+ false); >+if (TREE_CODE (base1) == INDIRECT_REF) >+ base1 = TREE_OPERAND (base1, 0); >+else >+ indirect_base1 = true; > } >-offset1 = TREE_OPERAND (arg1, 1); >-if (tree_fits_shwi_p (offset1)) >- { >-HOST_WIDE_INT off = size_low_cst (offset1); >-if ((HOST_WIDE_INT) (((unsigned HOST_WIDE_INT) off) >-
C++ PATCH for c++/67339 (ICE with alias to PMF)
We can't assume that a RECORD_TYPE has TYPE_LANG_SPECIFIC anymore. Tested x86_64-pc-linux-gnu, applying to trunk and 5. commit dafb66bbcf0fd6ef503e3573b2b315230d054c0e Author: Jason Merrill Date: Mon Dec 21 12:31:22 2015 -0500 PR c++/67339 * parser.c (cp_parser_elaborated_type_specifier): Use CLASS_TYPE_P rather than check for RECORD_TYPE. diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index c1948c4..262bfb2 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -16880,7 +16880,7 @@ cp_parser_elaborated_type_specifier (cp_parser* parser, { /* Indicate whether this class was declared as a `class' or as a `struct'. */ - if (TREE_CODE (type) == RECORD_TYPE) + if (CLASS_TYPE_P (type)) CLASSTYPE_DECLARED_CLASS (type) = (tag_type == class_type); cp_parser_check_class_key (tag_type, type); } diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-pmf1.C b/gcc/testsuite/g++.dg/cpp0x/alias-decl-pmf1.C new file mode 100644 index 000..d0ac27d --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-pmf1.C @@ -0,0 +1,16 @@ +// PR c++/67339 +// { dg-do compile { target c++11 } } + +template < typename T> +struct A +{ +void foo(); +template < typename S, typename W > +using N = void (T::*)(S, W) const ; +}; + +template < typename T> +void A::foo() +{ +typename A::template N fun = &T::out; +}
C++ PATCH for c++/67257 (ICE with broken template)
A template can declare a class template or a variable template, but not both at once. We were failing to diagnose this. Tested x86_64-pc-linux-gnu, applying to trunk. commit 316f3a84bf6f512e09b3d3aa72a1600f4cba0a75 Author: Jason Merrill Date: Mon Dec 21 13:46:09 2015 -0500 PR c++/67257 * parser.c (cp_parser_single_declaration): Reject a class template that also declares a variable. diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 262bfb2..842dded 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -25642,7 +25642,8 @@ cp_parser_single_declaration (cp_parser* parser, /* Check for the declaration of a template class. */ if (declares_class_or_enum) { - if (cp_parser_declares_only_class_p (parser)) + if (cp_parser_declares_only_class_p (parser) + || (declares_class_or_enum & 2)) { // If this is a declaration, but not a definition, associate // any constraints with the type declaration. Constraints @@ -25673,6 +25674,19 @@ cp_parser_single_declaration (cp_parser* parser, /* Perform access checks for template parameters. */ cp_parser_perform_template_parameter_access_checks (checks); + + /* Give a helpful diagnostic for + template struct A { } a; + if we aren't already recovering from an error. */ + if (!cp_parser_declares_only_class_p (parser) + && !seen_error ()) + { + error_at (cp_lexer_peek_token (parser->lexer)->location, + "a class template declaration must not declare " + "anything else"); + cp_parser_skip_to_end_of_block_or_statement (parser); + goto out; + } } } diff --git a/gcc/testsuite/g++.dg/cpp0x/pr51226.C b/gcc/testsuite/g++.dg/cpp0x/pr51226.C index 7e52e93..89200ee 100644 --- a/gcc/testsuite/g++.dg/cpp0x/pr51226.C +++ b/gcc/testsuite/g++.dg/cpp0x/pr51226.C @@ -6,4 +6,7 @@ template struct A // { dg-message "provided" } enum E : int; }; -template<> enum A<>::E : int {} // { dg-error "wrong number|expected" } +template<> enum A<>::E : int {} // { dg-error "wrong number" } + +// { dg-prune-output "expected" } +// { dg-prune-output "specialization" } diff --git a/gcc/testsuite/g++.dg/cpp1y/auto-fn28.C b/gcc/testsuite/g++.dg/cpp1y/auto-fn28.C new file mode 100644 index 000..fb3659b --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp1y/auto-fn28.C @@ -0,0 +1,11 @@ +// PR c++/67257 +// { dg-do compile { target c++14 } } + +template struct plus; +template struct A { + template auto operator()(T); +} foldl; // { dg-error "" } +void foo() { foldl>(0); } + +// { dg-prune-output "not declared" } +// { dg-prune-output "expected" }
C++ PATCH for c++/66921 (ICE with array in template)
An earlier patch of mine to prevent confusing array length deduction with a PMF constant was over-zealous; we should allow deduction from an already-deduced array type. Tested x86_64-pc-linux-gnu, applying to trunk, 5 and 4.9. commit 90ff39cf3d376c2359e6ecf9dc187907682a5f2a Author: Jason Merrill Date: Mon Dec 21 15:28:42 2015 -0500 PR c++/66921 * decl.c (cp_complete_array_type): Allow an initializer that already has array type. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index a14062b..af5f265 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -7479,7 +7479,8 @@ cp_complete_array_type (tree *ptype, tree initial_value, bool do_default) /* Don't get confused by a CONSTRUCTOR for some other type. */ if (initial_value && TREE_CODE (initial_value) == CONSTRUCTOR - && !BRACE_ENCLOSED_INITIALIZER_P (initial_value)) + && !BRACE_ENCLOSED_INITIALIZER_P (initial_value) + && TREE_CODE (TREE_TYPE (initial_value)) != ARRAY_TYPE) return 1; if (initial_value) diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array14.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array14.C new file mode 100644 index 000..b8eb084 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array14.C @@ -0,0 +1,9 @@ +// PR c++/66921 +// { dg-do compile { target c++11 } } + +template +struct Holder { + constexpr static const int array[] = { 1, 2, 3 }; + enum {F = array[0]}; +}; +class HI: public Holder {};
Re: [Patch, libstdc++/68877] Reimplement __is_[nothrow_]swappable
On 21/12/15 12:45 +0100, Daniel Krügler wrote: 2015-12-14 21:48 GMT+01:00 Daniel Krügler : This is a reimplementation of __is_swappable and __is_nothrow_swappable according to http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4511.html and also adds a missing usage of __is_nothrow_swappable in the swap overload for arrays. Strictly speaking the latter change differs from the Standard specification which requires the expression noexcept(swap(*a, *b)) to be used. On the other hand the Standard is broken in this regard, as pointed out by http://cplusplus.github.io/LWG/lwg-active.html#2554 The patch doesn't apply cleanly because it repeats some of the new files either twice or three times (and also has some trailing whitespace that shouldn't be there). After fixing the patch to only create new files once it applies, but then I get some FAILs: FAIL: 20_util/is_nothrow_swappable/value.cc (test for excess errors) FAIL: 20_util/is_swappable/value.cc (test for excess errors) I don't have time to analyse these today, so I'll wait until you're able to do so. There's no great rush to fix this, as long as it's in stage 3.
[PATCH] Avoid unnecessary block copying in path splitting
This is something Richi pointed out. I didn't think it was a big deal and didn't want to unnecessarily complicate the code like I did for jump threading to avoid a little bit of block copying. As it turns out avoiding the copying in path splitting is trivial, so trivial we actually get to remove a bit of code. To ensure this got tested, I put split-paths back into -O2 for my bootstrap & regression tested. It, of course, passed just fine. Installed on the trunk. Jeff commit ed304c35af7050ab30585fd0ad6ed968dfac73c1 Author: Jeff Law Date: Tue Dec 22 14:48:12 2015 -0700 [PATCH] Avoid unnecessary block copying in path splitting * gimple-ssa-split-paths.c (split_paths): Avoid unnecessary block copying. diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 73ab7d4..5e1e819 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,8 @@ +2015-12-22 Jeff Law + + * gimple-ssa-split-paths.c (split_paths): Avoid unnecessary block + copying. + 2015-12-22 Jakub Jelinek PR c++/67376 diff --git a/gcc/gimple-ssa-split-paths.c b/gcc/gimple-ssa-split-paths.c index 540fdf3..294a1af 100644 --- a/gcc/gimple-ssa-split-paths.c +++ b/gcc/gimple-ssa-split-paths.c @@ -192,9 +192,10 @@ split_paths () /* BB is the merge point for an IF-THEN-ELSE we want to transform. -Essentially we want to create two duplicates of BB and append -a duplicate to the THEN and ELSE clauses. This will split the -path leading to the latch. BB will be unreachable and removed. */ +Essentially we want to create a duplicate of bb and redirect the +first predecessor of BB to the duplicate (leaving the second +predecessor as is. This will split the path leading to the latch +re-using BB to avoid useless copying. */ if (bb && is_feasible_trace (bb)) { if (dump_file && (dump_flags & TDF_DETAILS)) @@ -202,9 +203,7 @@ split_paths () "Duplicating join block %d into predecessor paths\n", bb->index); basic_block pred0 = EDGE_PRED (bb, 0)->src; - basic_block pred1 = EDGE_PRED (bb, 1)->src; transform_duplicate (pred0, bb); - transform_duplicate (pred1, bb); changed = true; } }
Re: [Patch, libstdc++/68877] Reimplement __is_[nothrow_]swappable
2015-12-22 22:42 GMT+01:00 Jonathan Wakely : > On 21/12/15 12:45 +0100, Daniel Krügler wrote: >> >> 2015-12-14 21:48 GMT+01:00 Daniel Krügler : >>> >>> This is a reimplementation of __is_swappable and >>> __is_nothrow_swappable according to >>> >>> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4511.html >>> >>> and also adds a missing usage of __is_nothrow_swappable in the swap >>> overload for arrays. Strictly speaking the latter change differs from >>> the Standard specification which requires the expression >>> noexcept(swap(*a, *b)) to be used. On the other hand the Standard is >>> broken in this regard, as pointed out by >>> >>> http://cplusplus.github.io/LWG/lwg-active.html#2554 > > > The patch doesn't apply cleanly because it repeats some of the new > files either twice or three times (and also has some trailing > whitespace that shouldn't be there). After fixing the patch to only > create new files once it applies, but then I get some FAILs: > > FAIL: 20_util/is_nothrow_swappable/value.cc (test for excess errors) > FAIL: 20_util/is_swappable/value.cc (test for excess errors) > > I don't have time to analyse these today, so I'll wait until you're > able to do so. There's no great rush to fix this, as long as it's in > stage 3. Thanks for looking at this. When will stage 3 end? Thanks, - Daniel
Re: [RFC][PATCH] Fix broken handling of LABEL_REF in genrecog + genpreds.
Dominik Vogt writes: > On Fri, Dec 18, 2015 at 09:55:54AM +, Richard Sandiford wrote: >> Dominik Vogt writes: >> > The attached patch fixes the handling of LABEL_REF in genrecog and >> > genpreds. >> > >> > The current code assumes that X can have only a mode than PRED (X, >> > MODE) if X is CONST_INT, CONST_DOUBLE or CONST_WIDE_INT, but >> > actually that can be also the case for a LABEL_REF with VOIDmode. >> > Due to this it is necessary to add "const_int" (or similar) to >> > match_code of some predicates handling label_ref but not >> > const_int. >> >> OK, thanks. >> >> As mentioned in the other thread, I think LABEL_REFs shouldn't have >> VOIDmode, so long-term we should be fixing the targets. I agree this >> is the correct workaround until that happens though. > > All right, in the mean time the patch has passed the test suite on > x86_64, s390x and s390. See attached version 2 of the patch > (added just an additional comment in genpreds.c). This is OK too, thanks. "matches_const_scalar_int_p" is now a bit of a misnomer (it was supposed to be tied to CONST_SCALAR_INT_P) but I think we can live with that. (Especially since this is supposed to be "temporary"...) Richard
Re: Fix alias.c wrt aliases and anchors
Jan Hubicka writes: >> > Hi, >> > the alias-2.c testcase fails on targets with anchors. The reason is that >> > the variable itself is anchored while the alias is not and they point to >> > the >> > same location. I folllowed the docs of SYMBOL_REF claiming that >> > SYMBOL_REF_DECL if the symbol is label and tought it is safe to >> > disambiguate >> > them. >> >> What kind of distinction do you mean between "label" and non-label here? >> >> I don't think !SYMBOL_REF_DECL tells us anything useful about the >> symbol. I think it's more a case of: if SYMBOF_REF_DECL is present, >> we can assume that what it says is accurate. If it isn't present we >> can't assume anything. >> >> So was it... >> >> Jan Hubicka writes: >> > @@ -2323,26 +2367,14 @@ >> > >> >if (GET_CODE (x) == SYMBOL_REF && GET_CODE (y) == SYMBOL_REF) >> > { >> > - tree x_decl = SYMBOL_REF_DECL (x); >> > - tree y_decl = SYMBOL_REF_DECL (y); >> > - int cmp; >> > + int cmp = compare_base_symbol_refs (x,y); >> > >> > - if (!x_decl || !y_decl) >> > - { >> > -/* Label and normal symbol are never the same. */ >> > -if (x_decl != y_decl) >> > - return 0; >> > -return offset_overlap_p (c, xsize, ysize); >> >> ...this part of the code that was causing the problem? That doesn't >> seem valid even without the anchor stuff. I think the starting position > > Yep, I misread the docs assuming that SYMBOL_REF_DECL == NULL only > for code labels. It is always safe to disambiguate these as they access > readonly memory anyway. > >> should be that we can only use offset_overlap_p if XSTR (x) == XSTR (y) >> and need to assume a conflict otherwise. E.g. I think in principle it's >> valid for a target to create symbol_refs for different parts of a single >> artificial object. > > The code original was: > if (rtx_equal_for_memref_p (x, y)) > { > return offset_overlap_p (c, xsize, ysize); > } > and > rtx_equal_for_memref_p (const_rtx x, const_rtx y) > { > ... > case SYMBOL_REF: > return XSTR (x, 0) == XSTR (y, 0); > > So it assumed base+offset check for all labels with same XSTR. My patch > makes this strictly weaker: it is possible that the same variable is > accessed via its own symbol or via offset from variable anchor. Well, to me "weaker" means "makes more conservative assumptions", which in this context would be assuming a conflict in cases where the old code didn't (i.e. returning -1 more often). I'm not sure whether the first patch was strictly weaker in that sense, since if the symbol_refs were not equal according to rtx_equal_for_memref_p, the old code would fall through to the end of the function and return -1. >> I agree there are other refinements you can do on top of that, >> e.g. that two block symbols in different blocks can never conflict. >> But the patch seems to be treating anchors as an exception to the rule, >> whereas I think they're just one instance of the rule. > > Can you think of some testcase? Not a specific one, sorry, but it seems like the kind of thing that could happen with extra ABI-mandated constructs. But maybe the lack of a specific testcase is a good thing. If in practice anchors make up the vast majority of cases where (a) SYMBOL_REF_DECL is null and (b) XSTR is too weak, there should no harm in relying on the old XSTR comparison for the non-anchor null-decl cases. > Doing XSTR==XSTR test and assuming a conflict otherwise will cause a > conflict between > every external variable read/write and static variable read/write as one > will be anchored > and other not. Yeah, I think handling anchors is a good thing. It just seems that logically the correctness fix is to replace: /* Label and normal symbol are never the same. */ if (x_decl != y_decl) return 0; return offset_overlap_p (c, xsize, ysize); with something like: if (XSTR (x, 0) == XSTR (y, 0)) return offset_overlap_p (c, xsize, ysize); /* Symbols might conflict. */ return -1; Handling anchors would then be a very useful optimisation on top of that. Thanks, Richard
Re: [PATCH 0/4 v3] Fix PR/63679 when --param sra-max-scalarization-size specified
On Tue, 2015-12-22 at 17:36 +, Alan Lawrence wrote: > On 22/12/15 16:05, Bill Schmidt wrote: > > On Tue, 2015-12-22 at 15:54 +, Alan Lawrence wrote: > >> On 21/12/15 13:13, Alan Lawrence wrote: > >>> This is a respin of previous patch series: > >>> https://gcc.gnu.org/ml/gcc-patches/2015-10/msg03271.html > >>> Minus three of the smaller patches having already been committed; with the > >>> updated version of the main patch to SRA; and the patches to DOM reworked > >>> to avoid constructing gimple stmt's. > >>> > >>> IMHO this feels quite invasive for stage 3, however, I note the PR/63679 > >>> (ssa-dom-cse-2.c being XFAILed on a bunch of platforms, and currently > >>> still > >>> FAILing on ARM) is a regression from gcc 4.9.1. So I'd ask maintainer's > >>> thoughts > >>> as to whether we should take this patch set for gcc 6. > >>> > >>> > >> > >> I've now tested the series on powerpc64-none-linux-gnu (gcc110) and > >> powerpc64le-none-linux-gnu (gcc112). > >> > >> On powerpc64le, this causes the same guality failures as on AArch64: > >> gcc.dg/guality/pr54970.c -O1 line 15 a[0] == 1 > >> gcc.dg/guality/pr54970.c -O1 line 20 a[0] == 1 > >> gcc.dg/guality/pr54970.c -O1 line 25 a[0] == 1 > >> and the same with other optimization flags. (I note that there are quite a > >> lot > >> of guality failures on both powerpc and also aarch64, which are generally > >> not > >> included in the posts on the gcc-testresults mailing list). > > > > That's interesting. I never see these for powerpc64le on my internal > > build system. I wonder what the difference is? > > I've ssh'd into gcc112.fsffrance.org, a power8 powerpc64le system; > and configure --with-cpu=power8 --disable-libsanitizer --with-long-double-128 > --enable-languages=c,c++,lto. > > Hmmm...ISTR one variable that can make a big difference is the version (or > absence!) of gdb...gcc112 has gdb "Fedora 7.8.2-38.fc21", copyright 2014, and > GDB 7.8 looks to have been released at the end of 2014. I no longer use --disable-libsanitizer, but that doesn't seem relevant. I also use --disable-multilib, though I think thats the default for powerpc64le now; not sure. The GDB on my internal system is actually even older (7.7.1, from earlier in 2014), so I don't think that's necessarily to blame. Still seems mysterious. That said, for better or worse we've had a history of ignoring the guality failures for big-endian because we've seen so many of them, and they don't seem to reflect a real problem with the debugger. Understanding why they break hasn't managed to become a priority yet. I personally won't be upset if we have a few more than we did before, as it probably doesn't indicate anything relevant about your patch. Bill > > >> The same pr54970 tests still seem to pass on powerpc64 big-endian even > >> with the > >> patches. > > Ach, not quite. In fact those three are failing on powerpc64 bigendian even > *without* the patches (at all optimization levels besides -O0 where they > pass). > This is on gcc110, configure --enable-languages=c,c++,lto > --disable-libsanitizer > --with-cpu=power7 --with-long-double-128. GDB is Fedora 7.7.1-21.fc20, a bit > older, and I tested both unix/-m32 and unix/-m64 (following Bill Seurer's > posts > on gcc-testresults). > > I'll email a list of the failures I'm seeing offlist (summary: 186 on gcc112, > 148 on gcc110 with -m32, 395 on gcc112 with -m64); however, I suspect gdb > version is the difference we are looking for. > > Which *may* mean that with a more up-to-date GDB, it's possible those > failures > may not be introduced on ppc64le. (Or similarly AArch64.) Hmmm > > --Alan >
Re: [Patch, libstdc++/68877] Reimplement __is_[nothrow_]swappable
On 22/12/15 22:58 +0100, Daniel Krügler wrote: 2015-12-22 22:42 GMT+01:00 Jonathan Wakely : On 21/12/15 12:45 +0100, Daniel Krügler wrote: 2015-12-14 21:48 GMT+01:00 Daniel Krügler : This is a reimplementation of __is_swappable and __is_nothrow_swappable according to http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/n4511.html and also adds a missing usage of __is_nothrow_swappable in the swap overload for arrays. Strictly speaking the latter change differs from the Standard specification which requires the expression noexcept(swap(*a, *b)) to be used. On the other hand the Standard is broken in this regard, as pointed out by http://cplusplus.github.io/LWG/lwg-active.html#2554 The patch doesn't apply cleanly because it repeats some of the new files either twice or three times (and also has some trailing whitespace that shouldn't be there). After fixing the patch to only create new files once it applies, but then I get some FAILs: FAIL: 20_util/is_nothrow_swappable/value.cc (test for excess errors) FAIL: 20_util/is_swappable/value.cc (test for excess errors) I don't have time to analyse these today, so I'll wait until you're able to do so. There's no great rush to fix this, as long as it's in stage 3. Thanks for looking at this. When will stage 3 end? Probably in two or three months time.
[PATCH] c++/58109 - alignas() fails to compile with constant expression
The attached patch adds handling of dependent arguments to attribute aligned and attribute vector_size, fixing c++/58109 and 69022 - attribute vector_size ignored with dependent bytes. Tested on x86_64. Martin gcc/testsuite/ChangeLog: 2015-12-22 Martin Sebor PR c++/58109 PR c++/69022 * g++.dg/cpp0x/alignas5.C: New test. * g++.dg/ext/vector29.C: Same. gcc/cp/ChangeLog: 2015-12-22 Martin Sebor PR c++/58109 PR c++/69022 * decl2.c (is_late_template_attribute): Handle dependent argument to attribute align and attribute vector_size. Index: gcc/cp/decl2.c === --- gcc/cp/decl2.c (revision 231903) +++ gcc/cp/decl2.c (working copy) @@ -1183,6 +1183,16 @@ if (args && PACK_EXPANSION_P (args)) return true; + if (is_attribute_p ("aligned", name) + || is_attribute_p ("vector_size", name)) +{ + /* Attribute argument may be a dependent indentifier. */ + if (tree t = args ? TREE_VALUE (args) : NULL_TREE) + if (value_dependent_expression_p (t) + || type_dependent_expression_p (t)) + return true; +} + /* If any of the arguments are dependent expressions, we can't evaluate the attribute until instantiation time. */ for (arg = args; arg; arg = TREE_CHAIN (arg)) Index: gcc/testsuite/g++.dg/cpp0x/alignas5.C === --- gcc/testsuite/g++.dg/cpp0x/alignas5.C (revision 0) +++ gcc/testsuite/g++.dg/cpp0x/alignas5.C (working copy) @@ -0,0 +1,45 @@ +// PR c++/58109 - alignas() fails to compile with constant expression +// { dg-do compile } + +template +struct Base { + static const int Align = sizeof (T); +}; + +// Never instantiated. +template +struct Derived: Base +{ +#if __cplusplus >= 201102L + // This is the meat of the (simplified) regression test for c++/58109. + using B = Base; + using B::Align; + + alignas (Align) char a [1]; + alignas (Align) T b [1]; +#else + // Fake the test for C++ 98. +# define Align Base::Align +#endif + + char __attribute__ ((aligned (Align))) c [1]; + T __attribute__ ((aligned (Align))) d [1]; +}; + +// Instantiated to verify that the code is accepted even when instantiated. +template +struct InstDerived: Base +{ +#if __cplusplus >= 201102L + using B = Base; + using B::Align; + + alignas (Align) char a [1]; + alignas (Align) T b [1]; +#endif + + char __attribute__ ((aligned (Align))) c [1]; + T __attribute__ ((aligned (Align))) d [1]; +}; + +InstDerived dx; Index: gcc/testsuite/g++.dg/ext/vector29.C === --- gcc/testsuite/g++.dg/ext/vector29.C (revision 0) +++ gcc/testsuite/g++.dg/ext/vector29.C (working copy) @@ -0,0 +1,53 @@ +// PR c++/69022 - attribute vector_size ignored with dependent bytes +// { dg-do compile } + +template +struct A { static const int X = N; }; + +#if __cplusplus >= 201202L +# define ASSERT(e) static_assert (e, #e) +#else +# define ASSERT(e) \ + do { struct S { bool: !!(e); } asrt; (void)&asrt; } while (0) +#endif + +template +struct B: A +{ +#if __cplusplus >= 201202L + using A::X; +# define VecSize X +#else +# define VecSize A::X +#endif + +static void foo () +{ +char a __attribute__ ((vector_size (N))); +ASSERT (sizeof a == N); + +T b __attribute__ ((vector_size (N))); +ASSERT (sizeof b == N); +} + +static void bar () +{ +char c1 __attribute__ ((vector_size (VecSize))); +ASSERT (sizeof c1 == VecSize); + +char c2 __attribute__ ((vector_size (A::X))); +ASSERT (sizeof c2 == A::X); + +T d1 __attribute__ ((vector_size (VecSize))); +ASSERT (sizeof d1 == VecSize); + +T d2 __attribute__ ((vector_size (A::X))); +ASSERT (sizeof d2 == A::X); +} +}; + +void bar () +{ +B::foo (); +B::bar (); +}
[PATCH] c/68966 - atomic_fetch_* on atomic_bool not diagnosed
The attached patch rejects invocations of atomic fetch_op intrinsics on objects of _Bool type as discussed in the context of PR c/68908. Tested on x86_64. Martin gcc/testsuite/ChangeLog: 2015-12-22 Martin Sebor PR c/68966 * gcc.dg/atomic-fetch-bool.c: New test. * gcc.dg/sync-fetch-bool.c: Same. gcc/ChangeLog: 2015-12-22 Martin Sebor PR c/68966 * doc/extend.texi (__atomic Builtins, __sync Builtins): Document constraint on the type of arguments. gcc/c-family/ChangeLog: 2015-12-22 Martin Sebor PR c/68966 * c-common.c (sync_resolve_size): Reject first argument when it's a pointer to _Bool. Index: gcc/c-family/c-common.c === --- gcc/c-family/c-common.c (revision 231903) +++ gcc/c-family/c-common.c (working copy) @@ -7667,6 +7667,6 @@ if (error_operand_p (align)) return -1; if (TREE_CODE (align) != INTEGER_CST || !INTEGRAL_TYPE_P (TREE_TYPE (align))) { @@ -10657,11 +10658,15 @@ /* A helper function for resolve_overloaded_builtin in resolving the overloaded __sync_ builtins. Returns a positive power of 2 if the first operand of PARAMS is a pointer to a supported data type. - Returns 0 if an error is encountered. */ + Returns 0 if an error is encountered. + FETCH is true when FUNCTION is one of the _FETCH_ built-ins. */ static int -sync_resolve_size (tree function, vec *params) +sync_resolve_size (tree function, vec *params, bool fetch) { + /* Type of the argument. */ + tree argtype; + /* Type the argument points to. */ tree type; int size; @@ -10671,7 +10676,7 @@ return 0; } - type = TREE_TYPE ((*params)[0]); + argtype = type = TREE_TYPE ((*params)[0]); if (TREE_CODE (type) == ARRAY_TYPE) { /* Force array-to-pointer decay for C++. */ @@ -10686,12 +10691,16 @@ if (!INTEGRAL_TYPE_P (type) && !POINTER_TYPE_P (type)) goto incompatible; + if (fetch && TREE_CODE (type) == BOOLEAN_TYPE) + goto incompatible; + size = tree_to_uhwi (TYPE_SIZE_UNIT (type)); if (size == 1 || size == 2 || size == 4 || size == 8 || size == 16) return size; incompatible: - error ("incompatible type for argument %d of %qE", 1, function); + error ("operand type %qT is incompatible with argument %d of %qE", + argtype, 1, function); return 0; } @@ -11250,6 +11259,9 @@ vec *params) { enum built_in_function orig_code = DECL_FUNCTION_CODE (function); + + /* Is function is one of the _FETCH_ built-ins? */ + bool fetch_op = true; bool orig_format = true; tree new_return = NULL_TREE; @@ -11325,6 +11337,9 @@ case BUILT_IN_ATOMIC_COMPARE_EXCHANGE_N: case BUILT_IN_ATOMIC_LOAD_N: case BUILT_IN_ATOMIC_STORE_N: + { + fetch_op = false; + } case BUILT_IN_ATOMIC_ADD_FETCH_N: case BUILT_IN_ATOMIC_SUB_FETCH_N: case BUILT_IN_ATOMIC_AND_FETCH_N: @@ -11358,7 +11373,7 @@ case BUILT_IN_SYNC_LOCK_TEST_AND_SET_N: case BUILT_IN_SYNC_LOCK_RELEASE_N: { - int n = sync_resolve_size (function, params); + int n = sync_resolve_size (function, params, fetch_op); tree new_function, first_param, result; enum built_in_function fncode; Index: gcc/doc/extend.texi === --- gcc/doc/extend.texi (revision 231903) +++ gcc/doc/extend.texi (working copy) @@ -9246,6 +9246,8 @@ @{ tmp = *ptr; *ptr = ~(tmp & value); return tmp; @} // nand @end smallexample +The object pointed to by the first argument must of integer or pointer type. It must not be a Boolean type. + @emph{Note:} GCC 4.4 and later implement @code{__sync_fetch_and_nand} as @code{*ptr = ~(tmp & value)} instead of @code{*ptr = ~tmp & value}. @@ -9269,6 +9271,8 @@ @{ *ptr = ~(*ptr & value); return *ptr; @} // nand @end smallexample +The same constraints on arguments apply as for the corresponding @code{__sync_op_and_fetch} built-in functions. + @emph{Note:} GCC 4.4 and later implement @code{__sync_nand_and_fetch} as @code{*ptr = ~(*ptr & value)} instead of @code{*ptr = ~*ptr & value}. @@ -9515,13 +9519,13 @@ @deftypefnx {Built-in Function} @var{type} __atomic_or_fetch (@var{type} *ptr, @var{type} val, int memorder) @deftypefnx {Built-in Function} @var{type} __atomic_nand_fetch (@var{type} *ptr, @var{type} val, int memorder) These built-in functions perform the operation suggested by the name, and -return the result of the operation. That is, +return the result of the operation. That is, @smallexample @{ *ptr @var{op}= val; return *ptr; @} @end smallexample -All memory orders are valid. +The object pointed to by the first argument must of integer or pointer type. It must not be a Boolean type. All memory orders are valid. @end deftypefn @@ -9538,7 +9542,7 @@ @{ tmp = *ptr; *ptr @var{op}= val; return tmp; @} @end smallexample -All memory orders are valid. +The same constraints on arguments apply as for the corresponding @code{__at
Re: [Patch,tree-optimization]: Add new path Splitting pass on tree ssa representation
On 12/11/2015 02:11 AM, Ajit Kumar Agarwal wrote: Mibench/EEMBC benchmarks (Target Microblaze) Automotive_qsort1(4.03%), Office_ispell(4.29%), Office_stringsearch1(3.5%). Telecom_adpcm_d( 1.37%), ospfv2_lite(1.35%). I'm having a real tough time reproducing any of these results. In fact, I'm having a tough time seeing cases where path splitting even applies to the Mibench/EEMBC benchmarks mentioned above. In the very few cases where split-paths might apply, the net resulting assembly code I get is the same with and without split-paths. How consistent are these results? What functions are being affected that in turn impact performance? What options are you using to compile the benchmarks? I'm trying with -O2 -fsplit-paths and -O3 in my attempts to trigger the transformation so that I can look more closely at possible heuristics. Is this with the standard microblaze-elf target? Or with some other target? jeff
Re: [PATCH] C: fix reported range of invalid unary dereference.
On 12/22/2015 10:26 AM, David Malcolm wrote: Currently, trunk emits this for a bad unary * in C: bad-dereference.c:10:10: error: invalid type argument of unary ‘*’ (have ‘int’) return *some_f.x; ^ The following patch fixes the reported location to highlight the expression that was attempted to be dereferenced: bad-dereference.c:10:10: error: invalid type argument of unary ‘*’ (have ‘int’) return *some_f.x; ^ Based on another example from: http://clang.llvm.org/diagnostics.html albeit within the "Precision in Wording" example; I didn't change the wording. Successfully bootstrapped®rtested on x86_64-pc-linux-gnu. OK for trunk in stage 3? gcc/c/ChangeLog: * c-parser.c (c_parser_unary_expression): For dereferences, build a combined location before calling build_indirect_ref, so that error reports cover the full range, manually updating the c_expr src_range. gcc/testsuite/ChangeLog: * gcc.dg/bad-dereference.c: New test case. I agree with Trevor here, move those curlys out to enclose the entire switch case. It'll mean reformatting, but, meh... Approved with that change. jeff
Re: [PATCH] c++/58109 - alignas() fails to compile with constant expression
On Tue, 22 Dec 2015, Martin Sebor wrote: The attached patch adds handling of dependent arguments to attribute aligned and attribute vector_size, fixing c++/58109 and 69022 - attribute vector_size ignored with dependent bytes. In the longer term, Gaby used to suggest that internally we should represent the vector_size attribute as some kind of template __vector (half-way between a template class and a template alias). The goal would be to avoid duplicating all the logic from templates into attributes. Of course that's much more work (even assuming that there isn't some big road-block, which there might be), and a little bit more code duplication as in your patch seems appropriate at this stage. -- Marc Glisse