Re: New parameters to control stringop expansion libcall strategy
Updated patch attached (fixed header, buffer overflow, and warning -- error problems). Ok for trunk? thanks, David On Wed, Aug 7, 2013 at 6:04 PM, Joseph S. Myers jos...@codesourcery.com wrote: On Wed, 7 Aug 2013, Xinliang David Li wrote: Why the exception? This should only be used on the host, not the target. Sorry, I copied the boiler-plate header from i386.h -- is it wrong there too? tm.h gets included in target code because we haven't finished separating target macros used on the target from those used on the host. -- Joseph S. Myers jos...@codesourcery.com Index: config/i386/stringop.def === --- config/i386/stringop.def(revision 0) +++ config/i386/stringop.def(revision 0) @@ -0,0 +1,37 @@ +/* Definitions for stringop strategy for IA-32. + Copyright (C) 2013 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the files COPYING3. If not, +see http://www.gnu.org/licenses/. */ + +DEF_ENUM +DEF_ALG (no_stringop, no_stringop) +DEF_ENUM +DEF_ALG (libcall, libcall) +DEF_ENUM +DEF_ALG (rep_prefix_1_byte, rep_byte) +DEF_ENUM +DEF_ALG (rep_prefix_4_byte, rep_4byte) +DEF_ENUM +DEF_ALG (rep_prefix_8_byte, rep_8byte) +DEF_ENUM +DEF_ALG (loop_1_byte, byte_loop) +DEF_ENUM +DEF_ALG (loop, loop) +DEF_ENUM +DEF_ALG (unrolled_loop, unrolled_loop) +DEF_ENUM +DEF_ALG (vector_loop, vector_loop) Index: config/i386/stringop.opt === --- config/i386/stringop.opt(revision 0) +++ config/i386/stringop.opt(revision 0) @@ -0,0 +1,31 @@ +/* Definitions for stringop option handling for IA-32. + Copyright (C) 2013 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 3, or (at your option) +any later version. + +GCC is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU General Public License for more details. + +You should have received a copy of the GNU General Public License +along with GCC; see the files COPYING3. If not, +see http://www.gnu.org/licenses/. */ + +Enum(stringop_alg) String(rep_byte) Value(rep_prefix_1_byte) + +#undef DEF_ENUM +#define DEF_ENUM EnumValue + +#undef DEF_ALG +#define DEF_ALG(alg, name) Enum(stringop_alg) String(name) Value(alg) + +#include stringop.def + +#undef DEF_ENUM +#undef DEF_ALG Index: config/i386/i386-opts.h === --- config/i386/i386-opts.h (revision 201581) +++ config/i386/i386-opts.h (working copy) @@ -28,15 +28,17 @@ see the files COPYING3 and COPYING.RUNTI /* Algorithm to expand string function with. */ enum stringop_alg { - no_stringop, - libcall, - rep_prefix_1_byte, - rep_prefix_4_byte, - rep_prefix_8_byte, - loop_1_byte, - loop, - unrolled_loop, - vector_loop +#undef DEF_ENUM +#define DEF_ENUM + +#undef DEF_ALG +#define DEF_ALG(alg, name) alg, + +#include stringop.def +last_alg + +#undef DEF_ENUM +#undef DEF_ALG }; /* Available call abi. */ Index: config/i386/i386.c === --- config/i386/i386.c (revision 201582) +++ config/i386/i386.c (working copy) @@ -158,7 +158,7 @@ struct processor_costs ix86_size_cost = }; /* Processor costs (relative to an add) */ -static const +static struct processor_costs i386_cost = { /* 386 specific costs */ COSTS_N_INSNS (1), /* cost of an add instruction */ COSTS_N_INSNS (1), /* cost of a lea instruction */ @@ -228,7 +228,7 @@ struct processor_costs i386_cost = {/* 1, /* cond_not_taken_branch_cost. */ }; -static const +static struct processor_costs i486_cost = { /* 486 specific costs */ COSTS_N_INSNS (1), /* cost of an add instruction */ COSTS_N_INSNS (1), /* cost of a lea instruction */ @@ -300,7 +300,7 @@ struct processor_costs i486_cost = {/* 1, /* cond_not_taken_branch_cost. */ }; -static const +static struct processor_costs pentium_cost = { COSTS_N_INSNS (1), /* cost
Re: [PATCH] target/58065 ARM MALLOC_ABI_ALIGNMENT is wrong
On 08/07/13 08:10, Bernd Edlinger wrote: Hello, in the discussion about the PR middle-end/57748 it became obvious that the ARM target architecture should define a value for MALLOC_ABI_ALIGNMENT, because otherwise the default is simply word aligned, which causes sub-optimal code, at least for the neon fpu. This simple patch fixes PR target/58065 by defining the MALLOC_ABI_ALIGNMENT as BIGGEST_ALIGNMNET. As a proof that this has indeed some subtle influence on the generated code I created a new test case: The function foo is called by bar, and bar uses malloc to allocate the memory, with compiler options -O3 -g0 -mfpu=neon -mfloat-abi=softfp the function foo is inlined into bar, but the inlined version does not use vstr instructions any more, because the front-end does assume that malloc returns 4 byte aligned memory. You don't mention whether this patch was regression tested. All patches must be regression tested. I've applied only the changes to the source after testing it overnight with a couple of my other patches. Read further on about why this testcase is not suitable and how you could make this better for next time. For next time please read the contribution web page with respect to testing here http://gcc.gnu.org/contribute.html Regards Bernd Edlinger Now moving on to the patch itself - The testcase doesn't work because the vstr's that come out aren't because the alignment information doesn't match up rather than it being because the compiler has decided to use immediate offset addressing on storing a vector. Once you do that for a 128 bit vector there is no option but for the compiler to put out 2 vstr instructions which happens even after your patch for me. +++ gcc/testsuite/gcc.target/arm/pr58065.c 2013-08-03 00:08:22.0 +0200 @@ -0,0 +1,25 @@ +/* { dg-do compile } */ +/* { dg-options -O3 -g0 -mfpu=neon -mfloat-abi=softfp } */ For next time please use dg-add-options arm_neon for the -mfpu=neon -mfloat-abi=softfp bits. You also shouldn't need -g0 here. Instead I'd scan an intermedate rtl dump for alignment information rather than anything else. I haven't yet looked at the output but looking for A64 in an RTL dump instead of A32 might help in this particular case. Also reading http://gcc.gnu.org/wiki/TestCaseWriting will be useful with respect to creating new testcases. +/* { dg-final { scan-assembler-not \[^v]str } } */ + +#include stdlib.h + +typedef long long V + __attribute__ ((vector_size (2 * sizeof (long long)), may_alias)); + +struct T { V a; V b[1]; }; + +void +foo (struct T *p) +{ + V a = { 3, 4 }; + p-b[0] = a; +} + +struct T * +bar () +{ + struct T *t = (struct T *) malloc (sizeof(*t)); + foo (t); + return t; +} Please post the changelog inline in the future. 2013-08-07 Bernd Edlinger bernd.edlin...@hotmail.de PR target/58065 Set MALLOC_ABI_ALIGNMENT to BIGGEST_ALIGNMENT for ARM. * gcc/config/arm/arm.h: Define MALLOC_ABI_ALIGNMENT. * gcc/testsuite/gcc.target/arm/pr58065.c: New testcase. Also for next time there should be a changelog entry for each ChangeLog file you need to touch in this case with the file names in the changelog entry rooted at the file in which the ChangeLog file is present. In this particular case you need one for the source base and one for the testsuite and drop the gcc/ and gcc/testsuite prefixes to the file names below. Also notice how I've changed the Changelog to actually refer to MALLOC_ABI_ALIGNMENT rather than stating what's changed for arm.h. Therefore this should read 2013-08-08 Bernd Edlinger bernd.edlin...@hotmail.de PR target/58065 * config/arm/arm.h (MALLOC_ABI_ALIGNMENT): New, 2013-08-08 Bernd Edlinger bernd.edlin...@hotmail.de PR target/58065 * gcc.target/arm/pr58065.c: New testcase. This makes it easier for the person applying the patch to copy paste the Changelog into the relevant file. Thanks :) And finally thank you for submitting the patch, if you intend on contributing to GCC in the medium to longer term, it would be worth getting the process for obtaining a copyright assignment sorted if you haven't done so already. If you send a request for a copyright assignment form to g...@gcc.gnu.org, a maintainer who has access to the relevant forms will forward this on to you. regards Ramana
[PING][PATCH ARM]Extend thumb1_reorg to save more comparison instructions
Ping this patch, http://gcc.gnu.org/ml/gcc-patches/2013-04/msg01057.html Thanks. bin
RE: [PATCH][tree-optimization] Fix PR58088
Ok for trunk? Bootstrapped on x86_64-linux-gnu and tested arm-none-eabi on qemu. Thanks, Kyrill * gcc.c-torture/compile/pr58088.c: New test. Also, the ChangeLog entries should be: 2013-08-08 Kyrylo Tkachov kyrylo.tkac...@arm.com PR tree-optimization/58088 * gcc/fold-const.c (mask_with_trailing_zeros): New function. (fold_binary_loc): Make sure we don't recurse infinitely when the X in (X C1) | C2 is a tree of the form (Y * K1) K2. Use mask_with_trailing_zeros where appropriate. 2013-08-08 Kyrylo Tkachov kyrylo.tkac...@arm.com PR tree-optimization/58088 * gcc.c-torture/compile/pr58088.c: New test.
[Ping] Re: [PATCH, ARM] Fix unrecognizable vector comparisons
Ping? Is it OK for 4.8 and trunk? Thanks! -Zhenqiang On 1 August 2013 10:04, Zhenqiang Chen zhenqiang.c...@linaro.org wrote: Thank you all for the comments. The patch is updated as: 1) Revert it to the original one. 2) For the testcase, replace the dg-options with /* { dg-do compile } */ /* { dg-require-effective-target arm_neon } */ /* { dg-add-options arm_neon } */ /* { dg-options -O3 } */ Bootstrap on Chromebook and Pandaboard. No make check regression on Pandaboard. Thanks! -Zhenqiang On 9 July 2013 02:49, Jakub Jelinek ja...@redhat.com wrote: On Mon, Jul 08, 2013 at 11:44:04AM -0700, Janis Johnson wrote: @@ -0,0 +1,16 @@ +/* { dg-do compile } */ +/* { dg-options -O3 -mfpu=neon -mcpu=cortex-a9 -mthumb -mfloat-abi=hard -S } */ dg-add-options arm_neon ? dg-require-effective-target arm_neon ? I will update it. Please skip the test for multilibs whose flags include -mfpu or -mcpu options, which would conflict with or override the options in the test. Also the -S in dg-options looks wrong. That should be derived from dg-do. Jakub
Re: [PATCH, ARM] Fix unrecognizable vector comparisons
On 08/01/13 03:04, Zhenqiang Chen wrote: Thank you all for the comments. The patch is updated as: 1) Revert it to the original one. 2) For the testcase, replace the dg-options with /* { dg-do compile } */ /* { dg-require-effective-target arm_neon } */ /* { dg-add-options arm_neon } */ /* { dg-options -O3 } */ Bootstrap on Chromebook and Pandaboard. No make check regression on Pandaboard. Thanks! -Zhenqiang Ok for trunk and 4.8 branch. Sorry about the delay. Thanks, Ramana
Re: [PATCH][tree-optimization] Fix PR58088
Also, the ChangeLog entries should be: 2013-08-08 Kyrylo Tkachov kyrylo.tkac...@arm.com PR tree-optimization/58088 * gcc/fold-const.c (mask_with_trailing_zeros): New function. (fold_binary_loc): Make sure we don't recurse infinitely when the X in (X C1) | C2 is a tree of the form (Y * K1) K2. Use mask_with_trailing_zeros where appropriate. Without gcc/ prefix, paths are relative to the directory where ChangeLog lives. 2013-08-08 Kyrylo Tkachov kyrylo.tkac...@arm.com PR tree-optimization/58088 * gcc.c-torture/compile/pr58088.c: New test. This one is good. -- Eric Botcazou
Re: [buildbot] r201508: Build failures after pass C++ conversion
Sorry. How does the attached look? (am bootstrapping now) Thanks for devising the patch. However, we are in the process of fixing the issue on the Ada side so please do not apply it for now. -- Eric Botcazou
Re: [Patch] Whole regex refactoring and current status
Tim Shen timshe...@gmail.com writes: As metioned in this[1] email, I here propose a refactored version of regex, and show the status: This patch broke Solaris bootstrap: In file included from /usr/include/ctype.h:18:0, from /var/gcc/regression/trunk/10-gcc-gas/build/i386-pc-solaris2.10/libstdc++-v3/include/cctype:42, from /vol/gcc/src/hg/trunk/local/libstdc++-v3/include/precompiled/stdc++.h:35: /var/gcc/regression/trunk/10-gcc-gas/build/i386-pc-solaris2.10/libstdc++-v3/include/bits/regex.h:998:25: error: expected nested-name-specifier before numeric constant templatetypename _B, typename _A, typename _C, typename _R ^ and several more instances. This happens because iso/ctype_iso.h (via ctype.h) has: #define _B 0x0040 /* Blank */ #define _C 0x0020 /* Control character */ This is already documented: http://gcc.gnu.org/onlinedocs/libstdc++/manual/source_code_style.html#coding_style.bad_identifiers Please fix. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
RE: [Patch] Whole regex refactoring and current status
-Original Message- From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- ow...@gcc.gnu.org] On Behalf Of Rainer Orth Sent: 08 August 2013 11:02 To: Tim Shen Cc: libstdc++; gcc-patches Subject: Re: [Patch] Whole regex refactoring and current status Tim Shen timshe...@gmail.com writes: As metioned in this[1] email, I here propose a refactored version of regex, and show the status: This patch broke Solaris bootstrap: In file included from /usr/include/ctype.h:18:0, from /var/gcc/regression/trunk/10-gcc-gas/build/i386-pc- solaris2.10/libstdc++-v3/include/cctype:42, from /vol/gcc/src/hg/trunk/local/libstdc++- v3/include/precompiled/stdc++.h:35: /var/gcc/regression/trunk/10-gcc-gas/build/i386-pc-solaris2.10/libstdc++- v3/include/bits/regex.h:998:25: error: expected nested-name-specifier before numeric constant templatetypename _B, typename _A, typename _C, typename _R ^ and several more instances. This happens because iso/ctype_iso.h (via ctype.h) has: #define _B 0x0040 /* Blank */ #define _C 0x0020 /* Control character */ This is already documented: http://gcc.gnu.org/onlinedocs/libstdc++/manual/source_code_style.html#cod ing_style.bad_identifiers We're also seeing the same error for ARM builds. Kyrill
Re: [Patch] Whole regex refactoring and current status
Hi, This is already documented: http://gcc.gnu.org/onlinedocs/libstdc++/manual/source_code_style.html#coding_style.bad_identifiers Indeed. As a simple to remember rule never use single underscore + single uppercase. Usually we add a p, like _Cp, etc. Plenty of examples everywhere. At the moment I can't do the searchreplace, any such patch is preapproved though. Thanks, Paolo
Re: [PATCH/Merge Request] Vtable Verification feature.
Caroline, your libgcc ChangeLog entries are all broken: they lack the initial * as can easily be seen in Emacs' Change Log Mode. Please fix. As an aside, I had a very quick look at libvtv to determine what's required for a port to a non-Linux platform. It would be good if the requirements (currently, ELF, dl_iterate_phdr, __fortify_fail, certainly several more) could be documented to ease porters' task. Thanks. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH/Merge Request] Vtable Verification feature.
Hi Caroline, On 8 Aug 2013, at 11:27, Rainer Orth wrote: As an aside, I had a very quick look at libvtv to determine what's required for a port to a non-Linux platform. Likewise.. It would be good if the requirements (currently, ELF, dl_iterate_phdr, __fortify_fail, certainly several more) could be documented to ease porters' task. +1 Also, in general, it would be very helpful if folks introducing new named sections would use a target hook to fetch them - since section naming/flag conventions are different between (at least) elf and mach-o. thanks Iain
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
2013/8/8 Joseph S. Myers jos...@codesourcery.com: On Fri, 2 Aug 2013, Ilya Enkovich wrote: Hi All, I've updated MPX Wiki page (http://gcc.gnu.org/wiki/Intel%20MPX%20support%20in%20the%20GCC%20compiler). I added instrumentation description, programming model description, differences with other checkers, implementation details. Thanks. As noted, there should be a clean separation between what's generic and what's architecture-specific - the generic command-line options, hooks etc. shouldn't mention MPX in their names. That is not a big issue to rename generic names. But I'm just still trying to choose proper names. I looked into -fbounds-check but its description already mention C/C++ and its semantics differs from what new instrumentation does. I consider using -fcheck=pointer (currently valid for Fortran) and 'chkp' instead of 'mpx' for generic things. Does it look OK? I'm unclear on the references to *_nobnd and *_nochk functions - are there corresponding library (glibc etc.) changes to add additional function variants? Are all built-in functions that use pointers modified so that GCC will insert the required checks when expanding inline? The idea was to add new versions of some functions into glibc and replace normal versions call with special versions call when possible (e.g. when we know memcpy call does not copy pointers). Currently in MPX mode expanding is allowed for _nochk_nobnd variant which is equal to regular call. I cannot be sure new version will be added into glibc. If not, we would probably create additional library in GCC. I'm also unclear on how much --enable-mpx does - in general, it's desirable for a single compiler to be able to generate binaries both with and without the checks, and so quite possibly to build libgcc, libstdc++ etc. as multilibs both with and without MPX, rather than needing to make a static decision when GCC is built (so, any configure option should preferably be about building *extra* library variants, for example, rather than changing the options with which existing variants are built). Currently (in public mpx branch) --enable-mpx just adds compilation options to some libraries. I made an attempt to use multilibs instead. I tried to add mpx variant to target libraries build but got fail for libgfortran build. Does multilib support partial library rebuild? Actually I do not need libgfortran library (an many other libraries) to be in mpx version. Is it possible to get some libs from one place and some libs from another place? Thanks, Ilya -- Joseph S. Myers jos...@codesourcery.com
Re: PING: Re: [patch] implement simd loops in trunk (OMP_SIMD)
+ hash_table simduid_to_vf simduid_to_vf_htab; + hash_table decl_to_simduid decl_to_simduid_htab; Why two hashes? Seems like you can map from decl to vf directly. At what point do you have a simduid integer, but not the decl from whence it came? decl_to_simduid seems to be a misnomer. What it really is, is a omp_simd_array_to_simduid map. OMP simd array, being the temporary array used to hold simd lane private variable copies and such. The hash is not really mapping the simd DECL to the simduid, since that's just a matter of DECL_UID(simduid), but the OMP simd array to the index used to reference it (simduid), like thus: _7 = GOMP_SIMD_LANE (simduid.0) ... ... D.1737[_7] = stuff; decl_to_simduid is a mapping from the simduid.0 to the D.1737[] (the OMP simd array). Agreed, or am I missing something?
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
2013/8/8 Ilya Enkovich enkovich@gmail.com: 2013/8/8 Joseph S. Myers jos...@codesourcery.com: On Fri, 2 Aug 2013, Ilya Enkovich wrote: Hi All, I've updated MPX Wiki page (http://gcc.gnu.org/wiki/Intel%20MPX%20support%20in%20the%20GCC%20compiler). I added instrumentation description, programming model description, differences with other checkers, implementation details. Thanks. As noted, there should be a clean separation between what's generic and what's architecture-specific - the generic command-line options, hooks etc. shouldn't mention MPX in their names. That is not a big issue to rename generic names. But I'm just still trying to choose proper names. I looked into -fbounds-check but its description already mention C/C++ and its semantics differs from what new instrumentation does. I consider using -fcheck=pointer (currently valid for Fortran) and 'chkp' instead of 'mpx' for generic things. Does it look OK? I just realized that usage of option which is already defined for other languages may be problematic when this option is passed to MULTILIB_OPTIONS. So, probably, new common option -fcheck-pointers? Thanks, Ilya I'm unclear on the references to *_nobnd and *_nochk functions - are there corresponding library (glibc etc.) changes to add additional function variants? Are all built-in functions that use pointers modified so that GCC will insert the required checks when expanding inline? The idea was to add new versions of some functions into glibc and replace normal versions call with special versions call when possible (e.g. when we know memcpy call does not copy pointers). Currently in MPX mode expanding is allowed for _nochk_nobnd variant which is equal to regular call. I cannot be sure new version will be added into glibc. If not, we would probably create additional library in GCC. I'm also unclear on how much --enable-mpx does - in general, it's desirable for a single compiler to be able to generate binaries both with and without the checks, and so quite possibly to build libgcc, libstdc++ etc. as multilibs both with and without MPX, rather than needing to make a static decision when GCC is built (so, any configure option should preferably be about building *extra* library variants, for example, rather than changing the options with which existing variants are built). Currently (in public mpx branch) --enable-mpx just adds compilation options to some libraries. I made an attempt to use multilibs instead. I tried to add mpx variant to target libraries build but got fail for libgfortran build. Does multilib support partial library rebuild? Actually I do not need libgfortran library (an many other libraries) to be in mpx version. Is it possible to get some libs from one place and some libs from another place? Thanks, Ilya -- Joseph S. Myers jos...@codesourcery.com
Re: [Patch] Whole regex refactoring and current status
Hi Paolo, This is already documented: http://gcc.gnu.org/onlinedocs/libstdc++/manual/source_code_style.html#coding_style.bad_identifiers Indeed. As a simple to remember rule never use single underscore + single uppercase. Usually we add a p, like _Cp, etc. Plenty of examples everywhere. At the moment I can't do the searchreplace, any such patch is preapproved though. I wasn't certain about the right convention. The following patch allowed bootstrap to finish on i386-pc-solaris2.10 and x86_64-unknown-linux-gnu. I'll commit the patch once Solaris testing has finished. 2013-08-08 Rainer Orth r...@cebitec.uni-bielefeld.de * include/bits/regex.h: Replace _A, _B, _C, _R by _Ap, _Bp, _Cp, _Rp. diff --git a/libstdc++-v3/include/bits/regex.h b/libstdc++-v3/include/bits/regex.h --- a/libstdc++-v3/include/bits/regex.h +++ b/libstdc++-v3/include/bits/regex.h @@ -995,18 +995,18 @@ namespace std _GLIBCXX_VISIBILITY(defaul const basic_regex_CharT, _TraitsT, regex_constants::match_flag_type); - templatetypename _B, typename _A, typename _C, typename _R + templatetypename _Bp, typename _Ap, typename _Cp, typename _Rp friend bool -regex_match(_B, _B, -match_results_B, _A, -const basic_regex_C, _R, +regex_match(_Bp, _Bp, +match_results_Bp, _Ap, +const basic_regex_Cp, _Rp, regex_constants::match_flag_type); - templatetypename _B, typename _A, typename _C, typename _R + templatetypename _Bp, typename _Ap, typename _Cp, typename _Rp friend bool -regex_search(_B, _B, - match_results_B, _A, - const basic_regex_C, _R, +regex_search(_Bp, _Bp, + match_results_Bp, _Ap, + const basic_regex_Cp, _Rp, regex_constants::match_flag_type); flag_type _M_flags; @@ -2111,16 +2111,16 @@ namespace std _GLIBCXX_VISIBILITY(defaul templatetypename, typename, typename, typename friend class __detail::_BFSExecutor; - templatetypename _B, typename _A, typename _Ch_type, typename _Rx_traits + templatetypename _Bp, typename _Ap, typename _Ch_type, typename _Rx_traits friend bool -regex_match(_B, _B, match_results_B, _A, +regex_match(_Bp, _Bp, match_results_Bp, _Ap, const basic_regex_Ch_type, _Rx_traits, regex_constants::match_flag_type); - templatetypename _B, typename _A, typename _Ch_type, typename _Rx_traits + templatetypename _Bp, typename _Ap, typename _Ch_type, typename _Rx_traits friend bool -regex_search(_B, _B, match_results_B, _A, +regex_search(_Bp, _Bp, match_results_Bp, _Ap, const basic_regex_Ch_type, _Rx_traits, regex_constants::match_flag_type); However, I see many 32-bit testsuite failures, both on Solaris and Linux: Running target unix/-m32 FAIL: 28_regex/algorithms/regex_match/basic/string_01.cc (test for excess errors) WARNING: 28_regex/algorithms/regex_match/basic/string_01.cc compilation failed to produce executable FAIL: 28_regex/algorithms/regex_match/basic/string_range_00_03.cc (test for excess errors) WARNING: 28_regex/algorithms/regex_match/basic/string_range_00_03.cc compilation failed to produce executable FAIL: 28_regex/algorithms/regex_match/basic/string_range_01_03.cc (test for excess errors) WARNING: 28_regex/algorithms/regex_match/basic/string_range_01_03.cc compilation failed to produce executable FAIL: 28_regex/algorithms/regex_match/basic/string_range_02_03.cc (test for excess errors) WARNING: 28_regex/algorithms/regex_match/basic/string_range_02_03.cc compilation failed to produce executable FAIL: 28_regex/algorithms/regex_match/extended/53622.cc (test for excess errors) WARNING: 28_regex/algorithms/regex_match/extended/53622.cc compilation failed to produce executable FAIL: 28_regex/algorithms/regex_match/extended/57173.cc (test for excess errors) WARNING: 28_regex/algorithms/regex_match/extended/57173.cc compilation failed to produce executable FAIL: 28_regex/algorithms/regex_match/extended/cstring_bracket_01.cc (test for excess errors) WARNING: 28_regex/algorithms/regex_match/extended/cstring_bracket_01.cc compilation failed to produce executable FAIL: 28_regex/algorithms/regex_match/extended/cstring_plus.cc (test for excess errors) WARNING: 28_regex/algorithms/regex_match/extended/cstring_plus.cc compilation failed to produce executable FAIL: 28_regex/algorithms/regex_match/extended/cstring_questionmark.cc (test for excess errors) WARNING: 28_regex/algorithms/regex_match/extended/cstring_questionmark.cc compilation failed to produce executable FAIL:
Re: [Patch] Whole regex refactoring and current status
Hi, I wasn't certain about the right convention. The following patch allowed bootstrap to finish on i386-pc-solaris2.10 and x86_64-unknown-linux-gnu. I'll commit the patch once Solaris testing has finished. Thanks a lot. E.g. the first one is FAIL: 28_regex/algorithms/regex_match/basic/string_01.cc (test for excess errors) Excess errors: /vol/gcc/src/hg/trunk/local/libstdc++-v3/testsuite/28_regex/algorithms/regex_match/basic/string_01.cc:34:47: error: call of overloaded 'basic_regex(const char [8], const flag_type)' is ambiguous The other failures are similar. Tim, please address this ASAP, otherwise we have to revert the whole thing. Paolo
Re: [PATCH/Merge Request] Vtable Verification feature.
On 08/06/13 22:39, Benjamin De Kosnik wrote: +# Filter out unsupported systems. +case ${target} in + x86_64-*-linux* | i?86-*-linux*) + VTV_SUPPORTED=yes + ;; + powerpc*-*-linux*) + ;; + sparc*-*-linux*) + ;; + arm*-*-linux*) + ;; What about powerpc, sparc and arm? Why are they mentioned here if no actual decision is made about support? This is more a practical consideration: it's the middle of summer break. Let's error on the side of caution for the moment, and get this in causing minimal disruption on a convenient platform that I can verify myself easily. Once this is in trunk, let a million flowers bloom! There is no reason specific platform/target maintainers can't enable it at their leisure on a per-setup manner and when they can verify testresults easily. arm*-*-linux* is broken currently for what looks like the same reasons as the powerpc port. I spent some time this morning looking into what it would take to enable this for arm*-*-linux* today and the first thing that I noticed is that the testsuite is essentially driven by a shell script that caters to native testing on the only port that this was developed for. It also expects -m32 and -m64 to be a standard option in all ports that want to turn this on. Also because the tests are not using dejagnu it's going to be a right pain to get this to work for cross-testing and unless that works reliably one isn't going to be able to get this feature working easily. Also whatever is target specific in the testsuite like environment-fail-32.s and environment-fail-64.s needs to live in it's own target folders. Basically this feature needs to follow conventions that exist already in other parts of the source base or atleast have plans to get there surely ? I haven't seen this in the reviews that I've seen so far, so apologies if this has already been raised. regards Ramana
RE: [Patch] Whole regex refactoring and current status
Hi Paolo, This is already documented: http://gcc.gnu.org/onlinedocs/libstdc++/manual/source_code_style.html#c oding_style.bad_identifiers Indeed. As a simple to remember rule never use single underscore + single uppercase. Usually we add a p, like _Cp, etc. Plenty of examples everywhere. At the moment I can't do the searchreplace, any such patch is preapproved though. I wasn't certain about the right convention. The following patch allowed bootstrap to finish on i386-pc-solaris2.10 and x86_64-unknown-linux-gnu. I'll commit the patch once Solaris testing has finished. I can confirm that this patch fixes the arm-none-eabi build as well. Thanks, Kyrill
[PATCH, ARM] Fix handling of function arguments with excess alignment
PR target/56979 is a bug where a parameter to a function has an alignment that is larger than its natural alignment. In this case this causes the mid-end to generate a mode for the argument that is incompatible with the registers that are assigned for it. We then end up creating invalid RTL and subsequently abort when the pattern cannot emit assembly code. The fix is to decompose the assignment when this would happen in the same way that we handle other block mode arguments and handle each piece in turn. PR target/56979 * arm.c (aapcs_vfp_allocate): Decompose the argument if the suggested mode for the assignment isn't compatible with the registers required. Committed to trunk. R.--- arm.c (revision 201547) +++ arm.c (local) @@ -4544,7 +4544,9 @@ aapcs_vfp_allocate (CUMULATIVE_ARGS *pcu if (((pcum-aapcs_vfp_regs_free regno) mask) == mask) { pcum-aapcs_vfp_reg_alloc = mask regno; - if (mode == BLKmode || (mode == TImode !TARGET_NEON)) + if (mode == BLKmode + || (mode == TImode !TARGET_NEON) + || ! arm_hard_regno_mode_ok (FIRST_VFP_REGNUM + regno, mode)) { int i; int rcount = pcum-aapcs_vfp_rcount;
[PATCH, ARM] Fix ICE when using Neon vld1_dup_[su]64()
PR 57431 is an ICE when compiling for Neon intrinsics. For completeness vld1_dup_u64 is defined to fill a single element vector from a single element of data, but this is in reality degenerate. Unfortunately, by expanding this into a vec_duplicate operation we confuse the mid-end of the compiler, since it expects a vector mode to result from the duplicate operation. The fix is to special-case the expand into the appropriate direct load operation and to then simplify the logic for the remaining cases (which now really are all duplicate operations). PR target/57431 * arm/neon.md (neon_vld1_dupdi): New expand pattern. (neon_vld1_dupmode VD iterator): Iterate over VD not VDX. R.--- neon.md (revision 201427) +++ neon.md (local) @@ -4593,19 +4593,20 @@ (define_insn neon_vld1_lanemode ) (define_insn neon_vld1_dupmode - [(set (match_operand:VDX 0 s_register_operand =w) -(vec_duplicate:VDX (match_operand:V_elem 1 neon_struct_operand Um)))] + [(set (match_operand:VD 0 s_register_operand =w) +(vec_duplicate:VD (match_operand:V_elem 1 neon_struct_operand Um)))] TARGET_NEON -{ - if (GET_MODE_NUNITS (MODEmode) 1) -return vld1.V_sz_elem\t{%P0[]}, %A1; - else -return vld1.V_sz_elem\t%h0, %A1; -} - [(set (attr neon_type) - (if_then_else (gt (const_string V_mode_nunits) (const_string 1)) -(const_string neon_vld2_2_regs_vld1_vld2_all_lanes) -(const_string neon_vld1_1_2_regs)))] + vld1.V_sz_elem\t{%P0[]}, %A1 + [(set_attr neon_type neon_vld2_2_regs_vld1_vld2_all_lanes)] +) + +;; Special case for DImode. Treat it exactly like a simple load. +(define_expand neon_vld1_dupdi + [(set (match_operand:DI 0 s_register_operand ) +(unspec:DI [(match_operand:DI 1 neon_struct_operand )] + UNSPEC_VLD1))] + TARGET_NEON + ) (define_insn neon_vld1_dupmode
Re: [Patch] Whole regex refactoring and current status
On Thu, Aug 8, 2013 at 8:53 PM, Paolo Carlini paolo.carl...@oracle.com wrote: Tim, please address this ASAP, otherwise we have to revert the whole thing. I'm trying to reproduce the compilation failures. -- Tim Shen
Re: [Patch] Whole regex refactoring and current status
On Thu, Aug 8, 2013 at 10:04 PM, Tim Shen timshe...@gmail.com wrote: On Thu, Aug 8, 2013 at 8:53 PM, Paolo Carlini paolo.carl...@oracle.com wrote: Tim, please address this ASAP, otherwise we have to revert the whole thing. I'm trying to reproduce the compilation failures. There's a typedef in regex_constants.h: `typedef unsigned int syntax_option_type;` Which is a little bit naive. It possibly conflicts with size_t under i386 when overloading. I'm trying to change it to a bitset. Or is there any better solution? -- Tim Shen
Symtab cleanups 10/17 - ipa ref verifier
Hi, this patch implements (very basic) IPA REF verifier. It only checks if all references corresopnds to a real statement. It does not yet check if all statements have proper references attached to them. Even this simple testing found quite few positives where we leave stale references in callgraph. Fixed thus. Bootstrapped/regtested x86_64-linux, commited. Hope there won't be too much of other positives ;) * cgraphbuild.c (build_cgraph_edges): Do not walk into debugs. (make_pass_rebuild_cgraph_edges): Also clear references. * cgraph.c (verify_cgraph_node): Add basic ipa-ref verifier. * ipa-inline-transform.c (inline_transform): Remove all references after inlining. * cgraphunit.c (expand_function): Remove all references after expansion. * ipa-ref.c (ipa_ref_has_aliases_p): Fix formatting. (ipa_find_reference): Rewrite to iterator. (remove_stmt_references): Likewise. (ipa_clear_stmts_in_references): New function. * ipa-ref.h (ipa_clear_stmts_in_references): Declare. * cgraphclones.c (cgraph_materialize_all_clones): Remove or clear references. * ipa-split.c (split_function): Remove references in split function. Index: cgraphbuild.c === *** cgraphbuild.c (revision 201568) --- cgraphbuild.c (working copy) *** build_cgraph_edges (void) *** 318,323 --- 318,326 gimple stmt = gsi_stmt (gsi); tree decl; + if (is_gimple_debug (stmt)) + continue; + if (is_gimple_call (stmt)) { int freq = compute_call_stmt_bb_frequency (current_function_decl, *** make_pass_rebuild_cgraph_edges (gcc::con *** 537,543 static unsigned int remove_cgraph_callee_edges (void) { ! cgraph_node_remove_callees (cgraph_get_node (current_function_decl)); return 0; } --- 540,548 static unsigned int remove_cgraph_callee_edges (void) { ! struct cgraph_node *node = cgraph_get_node (current_function_decl); ! cgraph_node_remove_callees (node); ! ipa_remove_all_references (node-symbol.ref_list); return 0; } Index: cgraph.c === *** cgraph.c(revision 201568) --- cgraph.c(working copy) *** verify_cgraph_node (struct cgraph_node * *** 2537,2591 { if (this_cfun-cfg) { /* Reach the trees by walking over the CFG, and note the enclosing basic-blocks in the call edges. */ FOR_EACH_BB_FN (this_block, this_cfun) ! for (gsi = gsi_start_bb (this_block); ! !gsi_end_p (gsi); ! gsi_next (gsi)) ! { ! gimple stmt = gsi_stmt (gsi); ! if (is_gimple_call (stmt)) ! { ! struct cgraph_edge *e = cgraph_edge (node, stmt); ! tree decl = gimple_call_fndecl (stmt); ! if (e) ! { ! if (e-aux) ! { ! error (shared call_stmt:); ! cgraph_debug_gimple_stmt (this_cfun, stmt); ! error_found = true; ! } ! if (!e-indirect_unknown_callee) ! { ! if (verify_edge_corresponds_to_fndecl (e, decl)) ! { ! error (edge points to wrong declaration:); ! debug_tree (e-callee-symbol.decl); ! fprintf (stderr, Instead of:); ! debug_tree (decl); ! error_found = true; ! } ! } ! else if (decl) ! { ! error (an indirect edge with unknown callee ! corresponding to a call_stmt with ! a known declaration:); ! error_found = true; ! cgraph_debug_gimple_stmt (this_cfun, e-call_stmt); ! } ! e-aux = (void *)1; ! } ! else if (decl) ! { ! error (missing callgraph edge for call stmt:); ! cgraph_debug_gimple_stmt (this_cfun, stmt); ! error_found = true; ! } ! } } } else /* No CFG available?! */ --- 2537,2611 { if (this_cfun-cfg) { + pointer_set_t *stmts = pointer_set_create (); + int i; + struct ipa_ref *ref; + /* Reach the trees by walking
Re: [buildbot] r201508: Build failures after pass C++ conversion
This break Ada. Fixed thusly, bootstrapped and regtested on x86_64-suse-linux, applied. 2013-08-08 Eric Botcazou ebotca...@adacore.com * gcc-interface/Makefile.in (TOOLS_LIBS): Pick C object files from the compiler build and use standard library variables. (../../vxaddr2line$(exeext): Do not depend on targext.o and adjust. (gnatmake-re): Do not depend on targext.o. (gnatlink-re): Do not depend on link.o and targext.o. (../../gnatmake$(exeext): Likewise. (../../gnatlink$(exeext): Likewise. -- Eric Botcazou Index: gcc-interface/Makefile.in === --- gcc-interface/Makefile.in (revision 201177) +++ gcc-interface/Makefile.in (working copy) @@ -250,10 +250,9 @@ LIBS = $(LIBINTL) $(LIBICONV) $(LIBBACKT LIBDEPS = $(LIBINTL_DEP) $(LIBICONV_DEP) $(LIBBACKTRACE) $(LIBIBERTY) # Default is no TGT_LIB; one might be passed down or something TGT_LIB = -TOOLS_LIBS = targext.o link.o ../../ggc-none.o ../../libcommon-target.a \ +TOOLS_LIBS = ../link.o ../targext.o ../../ggc-none.o ../../libcommon-target.a \ ../../libcommon.a ../../../libcpp/libcpp.a $(LIBGNAT) $(LIBINTL) $(LIBICONV) \ - ../../../libbacktrace/.libs/libbacktrace.a ../../../libiberty/libiberty.a \ - $(SYSLIBS) $(TGT_LIB) + ../$(LIBBACKTRACE) ../$(LIBIBERTY) $(SYSLIBS) $(TGT_LIB) # Convert the target variable into a space separated list of architecture, # manufacturer, and operating system and assign each of those to its own @@ -2491,12 +2490,12 @@ common-tools: ../stamp-tools $(GNATBIND) $(ADA_INCLUDES) $(GNATBIND_FLAGS) gnatdll $(GNATLINK) -v gnatdll -o $@ --GCC=$(GCC_LINK) $(TOOLS_LIBS) -../../vxaddr2line$(exeext): ../stamp-tools targext.o +../../vxaddr2line$(exeext): ../stamp-tools $(GNATMAKE) -c $(ADA_INCLUDES) vxaddr2line --GCC=$(CC) $(ALL_ADAFLAGS) $(GNATBIND) $(ADA_INCLUDES) $(GNATBIND_FLAGS) vxaddr2line - $(GNATLINK) -v vxaddr2line -o $@ --GCC=$(GCC_LINK) targext.o $(CLIB) + $(GNATLINK) -v vxaddr2line -o $@ --GCC=$(GCC_LINK) ../targext.o $(CLIB) -gnatmake-re: ../stamp-tools link.o targext.o +gnatmake-re: ../stamp-tools $(GNATMAKE) -j0 $(ADA_INCLUDES) -u sdefault --GCC=$(CC) $(MOST_ADA_FLAGS) $(GNATMAKE) -j0 -c $(ADA_INCLUDES) gnatmake --GCC=$(CC) $(ALL_ADAFLAGS) $(GNATBIND) $(ADA_INCLUDES) $(GNATBIND_FLAGS) gnatmake @@ -2507,7 +2506,7 @@ gnatmake-re: ../stamp-tools link.o targe # with the former version of gnatlink itself which cannot override itself. # gnatlink-re cannot be run at the same time as gnatmake-re, hence the # dependency -gnatlink-re: ../stamp-tools link.o targext.o gnatmake-re +gnatlink-re: ../stamp-tools gnatmake-re $(GNATMAKE) -j0 -c $(ADA_INCLUDES) gnatlink --GCC=$(CC) $(ALL_ADAFLAGS) $(GNATBIND) $(ADA_INCLUDES) $(GNATBIND_FLAGS) gnatlink $(GNATLINK) -v gnatlink -o ../../gnatlinknew$(exeext) \ @@ -2519,11 +2518,11 @@ gnatlink-re: ../stamp-tools link.o targe # stamp target in the parent directory whenever gnat1 is rebuilt # Likewise for the tools -../../gnatmake$(exeext): $(P) b_gnatm.o link.o targext.o $(GNATMAKE_OBJS) +../../gnatmake$(exeext): $(P) b_gnatm.o $(GNATMAKE_OBJS) +$(GCC_LINK) $(ALL_CFLAGS) $(LDFLAGS) -o $@ b_gnatm.o $(GNATMAKE_OBJS) \ $(TOOLS_LIBS) -../../gnatlink$(exeext): $(P) b_gnatl.o link.o targext.o $(GNATLINK_OBJS) +../../gnatlink$(exeext): $(P) b_gnatl.o $(GNATLINK_OBJS) +$(GCC_LINK) $(ALL_CFLAGS) $(LDFLAGS) -o $@ b_gnatl.o $(GNATLINK_OBJS) \ $(TOOLS_LIBS)
Re: [buildbot] r201508: Build failures after pass C++ conversion
On Thu, 2013-08-08 at 16:36 +0200, Eric Botcazou wrote: This break Ada. Fixed thusly, bootstrapped and regtested on x86_64-suse-linux, applied. Thanks; sorry again about the breakage.
[PATCH][ARM] FAIL: gcc.target/arm/pr58041.c scan-assembler ldrb
Hi all, The recently added gcc.target/arm/pr58041.c test exposed a bug in the backend. When compiling for NEON and with -mno-unaligned-access we end up generating the vld1.64 and vst1.64 instructions instead of doing the accesses one byte at a time like -mno-unaligned-access expects. This patch fixes that by enabling the NEON expander and insns that produce these instructions only when unaligned accesses are allowed. Bootstrapped on arm-linux-gnueabihf. Tested arm-none-eabi on qemu. Ok for trunk and 4.8? Thanks, Kyrill 2013-08-08 Kyrylo Tkachov kyrylo.tkac...@arm.com * config/arm/neon.md (movmisalignmode): Disable when we don't allow unaligned accesses. (*movmisalignmode_neon_store): Likewise. (*movmisalignmode_neon_load): Likewise. (*movmisalignmode_neon_store): Likewise. (*movmisalignmode_neon_load): Likewise. neon_unaligned.patch Description: Binary data
Re: [buildbot] r201508: Build failures after pass C++ conversion
On Thu, 2013-08-08 10:43:53 -0400, David Malcolm dmalc...@redhat.com wrote: On Thu, 2013-08-08 at 16:36 +0200, Eric Botcazou wrote: This break Ada. Fixed thusly, bootstrapped and regtested on x86_64-suse-linux, applied. Thanks; sorry again about the breakage. Build Robot (http://toolchain.lug-owl.de/buildbot/) looks quite fine, too. Thanks for fixing it that fast! This was quite a major and intrusive change, so fallout is probably somewhat expected. It's probably even more a question of how it is handled afterwards, and I guess this was quite fine. MfG, JBG -- Jan-Benedict Glaw jbg...@lug-owl.de +49-172-7608481 Signature of: What we do for ourselves dies with us. What we do for the second : others and the world remains and is immortal. (Albert Pine) signature.asc Description: Digital signature
Re: [Patch, Fortran] PR 58058: [4.7/4.8/4.9 Regression] Memory leak with transfer function
ping! 2013/8/3 Janus Weil ja...@gcc.gnu.org: Hi all, the attached patch plugs a memory leak of the TRANSFER intrinsic, which can occur when transferring to CHARACTER strings. For details see the PR. Regtested on x86_64-unknown-linux-gnu. Ok for trunk/4.8/4.7? Cheers, Janus 2013-08-03 Janus Weil ja...@gcc.gnu.org PR fortran/58058 * trans-intrinsic.c (gfc_conv_intrinsic_transfer): Free the temporary string, if necessary. 2013-08-03 Janus Weil ja...@gcc.gnu.org PR fortran/58058 * gfortran.dg/transfer_intrinsic_6.f90: New.
Re: [Patch] Whole regex refactoring and current status
Hi, There's a typedef in regex_constants.h: `typedef unsigned int syntax_option_type;` Which is a little bit naive. It possibly conflicts with size_t under i386 when overloading. I'm trying to change it to a bitset. Or is there any better solution? In my humble opinion involving the whole std::bitset container for a syntax option is way overkill. Do you really have to do overloading between size_t and that type? Or maybe you can use a type *smaller* than unsigned int. Paolo
Re: New parameters to control stringop expansion libcall strategy
On Wed, 7 Aug 2013, Xinliang David Li wrote: Updated patch attached (fixed header, buffer overflow, and warning -- error problems). You still have diagnostics starting with a capital letter, contrary to the GNU Coding Standards. -- Joseph S. Myers jos...@codesourcery.com
Re: [Patch] Whole regex refactoring and current status
On Thu, Aug 8, 2013 at 11:14 PM, Paolo Carlini paolo.carl...@oracle.com wrote: In my humble opinion involving the whole std::bitset container for a syntax option is way overkill. Do you really have to do overloading between size_t and that type? Or maybe you can use a type *smaller* than unsigned int. The n3376 standard specified the following constructor: ... explicit basic_regex(const charT* p, flag_type f = regex_constants::ECMAScript); basic_regex(const charT* p, size_t len, flag_type f = regex_constants::ECMAScript); ... So flag_type shall not be the same as size_t. I don't know if when I switch flag_type from unsigned int to, say, unsigned short, conflicts will appear in 16bit archtectures. -- Tim Shen
Re: [PATCH, i386, MPX 1/X] Support of Intel MPX ISA
On Thu, 8 Aug 2013, Ilya Enkovich wrote: That is not a big issue to rename generic names. But I'm just still trying to choose proper names. I looked into -fbounds-check but its description already mention C/C++ and its semantics differs from what new instrumentation does. I consider using -fcheck=pointer (currently valid for Fortran) and 'chkp' instead of 'mpx' for generic things. Does it look OK? I just realized that usage of option which is already defined for other languages may be problematic when this option is passed to MULTILIB_OPTIONS. So, probably, new common option -fcheck-pointers? Seems reasonable to me. I made an attempt to use multilibs instead. I tried to add mpx variant to target libraries build but got fail for libgfortran build. Does multilib support partial library rebuild? Actually I do not need libgfortran library (an many other libraries) to be in mpx version. Is it possible to get some libs from one place and some libs from another place? I'm not sure why the libgfortran build would have failed ... maybe some libraries don't in fact do anything with pointers for which the checks would help, but if so then I'd expect the option simply not to have any effect on the code generated for those libraries. Multilibs are expected to be the same for all libraries (but packagers could no doubt optimize things in their packages, if in fact some libraries are identical when built both with and without MPX). -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH/Merge Request] Vtable Verification feature.
On Thu, 8 Aug 2013, Ramana Radhakrishnan wrote: I spent some time this morning looking into what it would take to enable this for arm*-*-linux* today and the first thing that I noticed is that the testsuite is essentially driven by a shell script that caters to native testing on the only port that this was developed for. It also expects -m32 and -m64 to be a standard option in all ports that want to turn this on. Also because the tests are not using dejagnu it's going to be a right pain to get this to work for cross-testing and unless that works reliably one isn't going to be able to get this feature working easily. Clearly it's got to be converted to DejaGnu (and must avoid depending on a build directory at all, installed testing with runtest --tool libvtv, having created an appropriate site.exp, should work as well). It's looking rather like this whole feature hasn't been adequately reviewed before commit -- Joseph S. Myers jos...@codesourcery.com
Re: [Patch] Whole regex refactoring and current status
Paolo Carlini paolo.carl...@oracle.com writes: In my humble opinion involving the whole std::bitset container for a syntax option is way overkill. It's already used for match_flag_type anyway. Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 And now for something completely different.
Re: Request to merge Undefined Behavior Sanitizer in
On Fri, 26 Jul 2013, Marc Glisse wrote: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57324 (that is using llvm's sanitizer) and for a first patch (unreviewed): http://gcc.gnu.org/ml/gcc-patches/2013-06/msg01466.html (started at http://gcc.gnu.org/ml/gcc-patches/2013-05/msg01402.html ) That patch is OK. Of course we'll want problems that show up when the testsuite is run with a compiler built with sanitization enabled to be fixed, as well as those that show up in a simple build of the compiler and its libraries, but it makes sense to fix the latter first. (And as the relevant functionality gets added to GCC's sanitizer, issues that show up in bootstrap with it enabled will need fixing as well.) -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Add vtable verification feature announcement to news on main page...
On 07/08/13 18:19, Caroline Tice wrote: As requested, here is the patch to announce the vtable verification feature on the main gcc.gnu.org web page. Is this ok to commit? Please don't put this in until the issues that the patch have introduced have been resolved. R.
Re: [Patch] Whole regex refactoring and current status
Hi, So flag_type shall not be the same as size_t. I don't know if when I switch flag_type from unsigned int to, say, unsigned short, conflicts will appear in 16bit archtectures. Well 16-bit archs aren't really supported these days, as long as the c++ runtime is concerned. Thus if unsigned short works let's go with it and move on. In any case, we don't have to make a super final decision, but breaking 32 bit is bad. In general, I would recommend being careful with adding templated containers, remember that eventually we want to have a lot of code in *.cc files and instantiations will be a big big annoyance. Paolo
Re: [Patch] Whole regex refactoring and current status
Hi, In my humble opinion involving the whole std::bitset container for a syntax option is way overkill. It's already used for match_flag_type anyway. Indeed, I noticed it a few days ago, and seemed overkill ;) Really, we already have implementation experience about these flags, eg in iostream, and I dont think we want std::bitset everywhere. Paolo Paolo
Re: [PATCH/Merge Request] Vtable Verification feature.
On Thu, Aug 8, 2013 at 5:55 AM, Ramana Radhakrishnan ramra...@arm.com wrote: On 08/06/13 22:39, Benjamin De Kosnik wrote: +# Filter out unsupported systems. +case ${target} in + x86_64-*-linux* | i?86-*-linux*) + VTV_SUPPORTED=yes + ;; + powerpc*-*-linux*) + ;; + sparc*-*-linux*) + ;; + arm*-*-linux*) + ;; What about powerpc, sparc and arm? Why are they mentioned here if no actual decision is made about support? This is more a practical consideration: it's the middle of summer break. Let's error on the side of caution for the moment, and get this in causing minimal disruption on a convenient platform that I can verify myself easily. Once this is in trunk, let a million flowers bloom! There is no reason specific platform/target maintainers can't enable it at their leisure on a per-setup manner and when they can verify testresults easily. arm*-*-linux* is broken currently for what looks like the same reasons as the powerpc port. Have you tried Benjamin Kosnik's patch? Does it fix the problem? I spent some time this morning looking into what it would take to enable this for arm*-*-linux* today and the first thing that I noticed is that the testsuite is essentially driven by a shell script that caters to native testing on the only port that this was developed for. It also expects -m32 and -m64 to be a standard option in all ports that want to turn this on. Also because the tests are not using dejagnu it's going to be a right pain to get this to work for cross-testing and unless that works reliably one isn't going to be able to get this feature working easily. Also whatever is target specific in the testsuite like environment-fail-32.s and environment-fail-64.s needs to live in it's own target folders. Basically this feature needs to follow conventions that exist already in other parts of the source base or atleast have plans to get there surely ? I haven't seen this in the reviews that I've seen so far, so apologies if this has already been raised. I know that the testsuite, as committed, is not in DejaGnu format and that it will need to be converted to DejaGnu format. I also know that it is currently specific to the platform on which the feature was developed. The plan has always been to fix this. There was some private discussion about whether it would be better to commit the testsuite in its current state or not to commit a testsuite at all with the initial project commit. I was told that it would be better to show that there is some way of testing this feature, even if the testsuite is not in its final form, than to not show any way of testing it at all. I will work on getting the testsuite into a better form and also on writing up some documentation on the platform-specific pieces of the vtable verification feature (for those who wish to port it to other platforms).
Re: New parameters to control stringop expansion libcall strategy
Updated. thanks, David On Thu, Aug 8, 2013 at 8:18 AM, Joseph S. Myers jos...@codesourcery.com wrote: On Wed, 7 Aug 2013, Xinliang David Li wrote: Updated patch attached (fixed header, buffer overflow, and warning -- error problems). You still have diagnostics starting with a capital letter, contrary to the GNU Coding Standards. -- Joseph S. Myers jos...@codesourcery.com Index: testsuite/gcc.target/i386/memcpy-strategy-1.c === --- testsuite/gcc.target/i386/memcpy-strategy-1.c (revision 0) +++ testsuite/gcc.target/i386/memcpy-strategy-1.c (revision 0) @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options -O2 -march=atom -mmemcpy-strategy=vector_loop:-1:align } */ +/* { dg-final { scan-assembler-times movdqa 8 { target { ! { ia32 } } } } } */ +/* { dg-final { scan-assembler-times movdqa 4 { target { ia32 } } } } */ + +char a[2048]; +char b[2048]; +void t (void) +{ + __builtin_memcpy (a, b, 2048); +} + Index: testsuite/gcc.target/i386/memcpy-strategy-2.c === --- testsuite/gcc.target/i386/memcpy-strategy-2.c (revision 0) +++ testsuite/gcc.target/i386/memcpy-strategy-2.c (revision 0) @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options -O2 -march=atom -mmemcpy-strategy=vector_loop:3000:align,libcall:-1:align } */ +/* { dg-final { scan-assembler-times movdqa 8 { target { ! { ia32 } } } } } */ +/* { dg-final { scan-assembler-times movdqa 4 { target { ia32 } } } } */ + +char a[2048]; +char b[2048]; +void t (void) +{ + __builtin_memcpy (a, b, 2048); +} + Index: testsuite/gcc.target/i386/memset-strategy-1.c === --- testsuite/gcc.target/i386/memset-strategy-1.c (revision 0) +++ testsuite/gcc.target/i386/memset-strategy-1.c (revision 0) @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options -O2 -march=atom -mmemset-strategy=libcall:-1:align } */ +/* { dg-final { scan-assembler-times memset 2 } } */ + +char a[2048]; +void t (void) +{ + __builtin_memset (a, 1, 2048); +} + Index: testsuite/gcc.target/i386/memcpy-strategy-3.c === --- testsuite/gcc.target/i386/memcpy-strategy-3.c (revision 0) +++ testsuite/gcc.target/i386/memcpy-strategy-3.c (revision 0) @@ -0,0 +1,10 @@ +/* { dg-do compile } */ +/* { dg-options -O2 -march=atom -mmemcpy-strategy=vector_loop:2000:align,libcall:-1:align } */ +/* { dg-final { scan-assembler-times memcpy 2 } } */ + +char a[2048]; +char b[2048]; +void t (void) +{ + __builtin_memcpy (a, b, 2048); +} Index: doc/invoke.texi === --- doc/invoke.texi (revision 201581) +++ doc/invoke.texi (working copy) @@ -652,6 +652,7 @@ Objective-C and Objective-C++ Dialects}. -mbmi2 -mrtm -mlwp -mthreads @gol -mno-align-stringops -minline-all-stringops @gol -minline-stringops-dynamically -mstringop-strategy=@var{alg} @gol +-mmemcpy-strategy=@var{strategy} -mmemset-strategy=@var{strategy} -mpush-args -maccumulate-outgoing-args -m128bit-long-double @gol -m96bit-long-double -mlong-double-64 -mlong-double-80 @gol -mregparm=@var{num} -msseregparm @gol @@ -14651,6 +14652,24 @@ Expand into an inline loop. Always use a library call. @end table +@item -mmemcpy-strategy=@var{strategy} +@opindex mmemcpy-strategy=@var{strategy} +Override the internal decision heuristic to decide if @code{__builtin_memcpy} +should be inlined and what inline algorithm to use when the expected size +of the copy operation is known. @var{strategy} +is a comma-separated list of @var{alg}:@var{max_size}:@var{dest_align} triplets. +@var{alg} is specified in @option{-mstringop-strategy}, @var{max_size} specifies +the max byte size with which inline algorithm @var{alg} is allowed. For the last +triplet, the @var{max_size} must be @code{-1}. The @var{max_size} of the triplets +in the list must be specified in increasing order. The minimal byte size for +@var{alg} is @code{0} for the first triplet and @code{@var{max_size} + 1} of the +preceding range. + +@item -mmemset-strategy=@var{strategy} +@opindex mmemset-strategy=@var{strategy} +The option is similar to @option{-mmemcpy-strategy=} except that it is to control +@code{__builtin_memset} expansion. + @item -momit-leaf-frame-pointer @opindex momit-leaf-frame-pointer Don't keep the frame pointer in a register for leaf functions. This Index: config/i386/stringop.def === --- config/i386/stringop.def(revision 0) +++ config/i386/stringop.def(revision 0) @@ -0,0 +1,37 @@ +/* Definitions for stringop strategy for IA-32. + Copyright (C) 2013 Free Software Foundation, Inc. + +This file is part of GCC. + +GCC is free software; you can redistribute it and/or modify +it under the terms of the GNU General
Re: [PATCH/Merge Request] Vtable Verification feature.
On Thu, Aug 8, 2013 at 3:27 AM, Rainer Orth r...@cebitec.uni-bielefeld.de wrote: Caroline, your libgcc ChangeLog entries are all broken: they lack the initial * as can easily be seen in Emacs' Change Log Mode. Please fix. Wow, I'm really sorry about that! I don't know how that happened. I'll fix it immediately. As an aside, I had a very quick look at libvtv to determine what's required for a port to a non-Linux platform. It would be good if the requirements (currently, ELF, dl_iterate_phdr, __fortify_fail, certainly several more) could be documented to ease porters' task. I will be working on some documentation for this. Thanks. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: PING: Re: [patch] implement simd loops in trunk (OMP_SIMD)
On 08/08/2013 02:06 AM, Aldy Hernandez wrote: The hash is not really mapping the simd DECL to the simduid, since that's just a matter of DECL_UID(simduid), but the OMP simd array to the index used to reference it (simduid), like thus: _7 = GOMP_SIMD_LANE (simduid.0) ... ... D.1737[_7] = stuff; decl_to_simduid is a mapping from the simduid.0 to the D.1737[] (the OMP simd array). Agreed, or am I missing something? Agreed. Yeah, that's a bit confusing. r~
Re: [PATCH][ARM] FAIL: gcc.target/arm/pr58041.c scan-assembler ldrb
On 08/08/13 15:44, Kyrylo Tkachov wrote: Hi all, The recently added gcc.target/arm/pr58041.c test exposed a bug in the backend. When compiling for NEON and with -mno-unaligned-access we end up generating the vld1.64 and vst1.64 instructions instead of doing the accesses one byte at a time like -mno-unaligned-access expects. This patch fixes that by enabling the NEON expander and insns that produce these instructions only when unaligned accesses are allowed. Bootstrapped on arm-linux-gnueabihf. Tested arm-none-eabi on qemu. Ok for trunk and 4.8? Ok for trunk and 4.8 . regards Ramana Thanks, Kyrill 2013-08-08 Kyrylo Tkachov kyrylo.tkac...@arm.com * config/arm/neon.md (movmisalignmode): Disable when we don't allow unaligned accesses. (*movmisalignmode_neon_store): Likewise. (*movmisalignmode_neon_load): Likewise. (*movmisalignmode_neon_store): Likewise. (*movmisalignmode_neon_load): Likewise.
Re: [Patch] Whole regex refactoring and current status
On Fri, Aug 9, 2013 at 12:15 AM, Paolo Carlini paolo.carl...@oracle.com wrote: Indeed, I noticed it a few days ago, and seemed overkill ;) Really, we already have implementation experience about these flags, eg in iostream, and I dont think we want std::bitset everywhere. So here's the change. It's under testing now, but could took several hours. If someone has a faster machine, please tell the result :) As a simple test, `g++ -m32 test.cc` didn't fail. -- Tim Shen a.patch Description: Binary data
Re: [PING] [PATCH, AArch64] Skip gcc.dg/lower-subreg-1.c
Ping~ Thanks, Yufeng On 07/26/13 12:06, Yufeng Zhang wrote: Hi, This patch changes to skip gcc.dg/lower-subreg-1.c for aarch64*-*-*. The word mode in aarch64 is 64-bit so the lower-subreg pass won't happen in this test case. The test is currently skipped on aarch64 with lp64 due to the directive of dg-require-effective-target ilp32, but fails when -mabi=ilp32 is in use. OK to commit? Thanks, Yufeng gcc/testsuite/ * gcc.dg/lower-subreg-1.c: Skip aarch64*-*-*.
Re: [Patch] Whole regex refactoring and current status
On 08/08/2013 06:51 PM, Tim Shen wrote: On Fri, Aug 9, 2013 at 12:15 AM, Paolo Carlini paolo.carl...@oracle.com wrote: Indeed, I noticed it a few days ago, and seemed overkill ;) Really, we already have implementation experience about these flags, eg in iostream, and I dont think we want std::bitset everywhere. So here's the change. It's under testing now, but could took several hours. If someone has a faster machine, please tell the result :) As a simple test, `g++ -m32 test.cc` didn't fail. Thanks! Paolo.
Re: [C++ Patch / RFC] PR 46206
I'm now seeing a new testsuite failure: FAIL: g++.dg/lookup/typedef2.C -std=c++98 (test for excess errors) Excess errors: /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:8:12: error: using typedef-name 'Foo1::Bar' after 'struct' /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:8:19: error: invalid type in declaration before ';' token /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:18:12: error: using typedef-name 'Foo2::Bar' after 'struct' /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:18:19: error: invalid type in declaration before ';' token Thanks, David
Re: [C++ Patch / RFC] PR 46206
On 08/08/2013 07:35 PM, David Edelsohn wrote: I'm now seeing a new testsuite failure: FAIL: g++.dg/lookup/typedef2.C -std=c++98 (test for excess errors) Excess errors: /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:8:12: error: using typedef-name 'Foo1::Bar' after 'struct' /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:8:19: error: invalid type in declaration before ';' token /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:18:12: error: using typedef-name 'Foo2::Bar' after 'struct' /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:18:19: error: invalid type in declaration before ';' token Which essentially means that my patchlet fixes the problem on x86_64-linux, but not on AIX. Because you would see the exact same fails if I revert the patch and keep the testcase in. I'm ready to revert the patch and re-opening or xfail-ing on AIX. Jason call I guess. Paolo.
Re: [C++ Patch / RFC] PR 46206
Why does the patch and fix have any architecture or OS-dependency? - David On Thu, Aug 8, 2013 at 1:57 PM, Paolo Carlini paolo.carl...@oracle.com wrote: On 08/08/2013 07:35 PM, David Edelsohn wrote: I'm now seeing a new testsuite failure: FAIL: g++.dg/lookup/typedef2.C -std=c++98 (test for excess errors) Excess errors: /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:8:12: error: using typedef-name 'Foo1::Bar' after 'struct' /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:8:19: error: invalid type in declaration before ';' token /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:18:12: error: using typedef-name 'Foo2::Bar' after 'struct' /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:18:19: error: invalid type in declaration before ';' token Which essentially means that my patchlet fixes the problem on x86_64-linux, but not on AIX. Because you would see the exact same fails if I revert the patch and keep the testcase in. I'm ready to revert the patch and re-opening or xfail-ing on AIX. Jason call I guess. Paolo.
Re: [C++ Patch / RFC] PR 46206
On 08/08/2013 08:18 PM, David Edelsohn wrote: Why does the patch and fix have any architecture or OS-dependency? It's tricky, but it depends on the way are internally stored and handled *multiple* TYPE_DECL for essentially the same typedef. The class layout must involve both such typedef and a few data members. I had a look to testresults, and the patch appears to work for other -linux targets too, not just x86_64-linux, and it's elementary, doesn't add complexity to the code, thus unless it proves to cause regressions on the targets where it appears to work, it would be a pity to revert it. But as I said the issue is weird, after all remained unfixed for 3-4 years, realistically, I don't know if together with all the other issues I have in my TODO, I can promise to understand it in such detail to fix it on AIX too. Paolo.
Re: [PATCH/Merge Request] Vtable Verification feature.
arm*-*-linux* is broken currently for what looks like the same reasons as the powerpc port. Have you tried Benjamin Kosnik's patch? Does it fix the problem? I hadn't but that seems to have fixed the issue. Thanks. I spent some time this morning looking into what it would take to enable this for arm*-*-linux* today and the first thing that I noticed is that the testsuite is essentially driven by a shell script that caters to native testing on the only port that this was developed for. It also expects -m32 and -m64 to be a standard option in all ports that want to turn this on. Also because the tests are not using dejagnu it's going to be a right pain to get this to work for cross-testing and unless that works reliably one isn't going to be able to get this feature working easily. Also whatever is target specific in the testsuite like environment-fail-32.s and environment-fail-64.s needs to live in it's own target folders. Basically this feature needs to follow conventions that exist already in other parts of the source base or atleast have plans to get there surely ? I haven't seen this in the reviews that I've seen so far, so apologies if this has already been raised. I know that the testsuite, as committed, is not in DejaGnu format and that it will need to be converted to DejaGnu format. I also know that it is currently specific to the platform on which the feature was developed. The plan has always been to fix this. There was some private discussion about whether it would be better to commit the testsuite in its current state or not to commit a testsuite at all with the initial project commit. I was told that it would be better to show that there is some way of testing this feature, even if the testsuite is not in its final form, than to not show any way of testing it at all. Good to know there is atleast a plan to fix this. It would have been better to state this in a public review or as part of your merge request or indeed fix this properly before merging . I will work on getting the testsuite into a better form and also on writing up some documentation on the platform-specific pieces of the vtable verification feature (for those who wish to port it to other platforms). Cool - thanks . So I tried playing with it on native arm*-*-*linux builds for sometime and ran into the first problem which came while just building vtv_start.c . /bin/bash ./libtool --tag=CC --mode=compile /home/ramrad01/build-vtv1/./gcc/xgcc -B/home/ramrad01/build-vtv1/./gcc/ -B/usr/local/armv7l-unknown-linux-gnueabihf/bin/ -B/usr/local/armv7l-unknown-linux-gnueabihf/lib/ -isystem /usr/local/armv7l-unknown-linux-gnueabihf/include -isystem /usr/local/armv7l-unknown-linux-gnueabihf/sys-include -I. -I../../../gcc/libvtv -I../../../gcc/libvtv/../include -D_GNU_SOURCE -Wall -Wextra -fno-exceptions -g -O2 -MT vtv_start.lo -MD -MP -MF .deps/vtv_start.Tpo -c -o vtv_start.lo vtv_start.c libtool: compile: /home/ramrad01/build-vtv1/./gcc/xgcc -B/home/ramrad01/build-vtv1/./gcc/ -B/usr/local/armv7l-unknown-linux-gnueabihf/bin/ -B/usr/local/armv7l-unknown-linux-gnueabihf/lib/ -isystem /usr/local/armv7l-unknown-linux-gnueabihf/include -isystem /usr/local/armv7l-unknown-linux-gnueabihf/sys-include -I. -I../../../gcc/libvtv -I../../../gcc/libvtv/../include -D_GNU_SOURCE -Wall -Wextra -fno-exceptions -g -O2 -MT vtv_start.lo -MD -MP -MF .deps/vtv_start.Tpo -c vtv_start.c -fPIC -DPIC -o .libs/vtv_start.o vtv_start.c:57:1: warning: constructor priorities from 0 to 100 are reserved for the implementation [enabled by default] { ^ vtv_start.c:65:3: internal compiler error: Segmentation fault = { }; ^ 0x3aa3f5 crash_signal ../../gcc/gcc/toplev.c:335 0x52a669 default_elf_asm_named_section(char const*, unsigned int, tree_node*) ../../gcc/gcc/varasm.c:6219 0x52c041 switch_to_section ../../gcc/gcc/varasm.c:7035 0x52c5b1 output_object_block ../../gcc/gcc/varasm.c:7209 0x52c5b1 output_object_block_htab ../../gcc/gcc/varasm.c:7270 Please submit a full bug report, with preprocessed source if appropriate. Please include the complete backtrace with any bug report. See http://gcc.gnu.org/bugs.html for instructions. make[2]: *** [vtv_start.lo] Error 1 make[2]: Leaving directory `/home/ramrad01/build-vtv1/armv7l-unknown-linux-gnueabihf/libvtv' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/home/ramrad01/build-vtv1/armv7l-unknown-linux-gnueabihf/libvtv' make: *** [all] Error 2 It might be because of inconsistencies for handling comdat linkage with regards to the ARM C++ ABI variations but it's too late for me to dig further tonight. regards Ramana
Re: [patch, mips] Add nan2008 multilibs to mips-mti-* targets.
Steve Ellcey sell...@mips.com writes: I would like to add new variations to the mips-mti-elf and mips-mti-linux targets to support the new NaN format on MIPS (mnan=2008 for IEEE 754-2008). While I was doing this I noticed that I was handling mips16 and micromips in a dumb manner. I had them in MULTILIB_OPTIONS as non-exclusive flags and then used MULTILIB_EXCEPTIONS to not allow them to both be used. So I also changed MULTILIB_OPTIONS to make mips16 and micromips exclusive and then I could get rid of one of the MULTILIB_EXCEPTIONS entries. Good spot. Looks like you could do the same thing with -mabi=64, but it isn't as logical as the mips16/micromips thing and probably wouldn't win much. I also make the new mnan=2008 flag exclusive of msoft-float since it is only useful for hard-float. Well, it's useful for both, because -msoft-float uses the same NaN format as -mhard-float. But not having the multilib is fine. 2013-08-07 Steve Ellcey sell...@mips.com * config/mips/mti-linux.h (SYSROOT_SUFFIX_SPEC): Add nan2008. * config/mips/t-mti-elf (MULTILIB_OPTIONS): Make mips16 and micromips incompatible. Add nan2008. (MULTILIB_DIRNAMES): Add nan2008. (MULTILIB_EXCEPTIONS): Remove mips16/micromips entry. * config/mips/t-mti-linux (MULTILIB_OPTIONS): Make mips16 and micromips incompatible. Add nan2008. (MULTILIB_DIRNAMES): Add nan2008. (MULTILIB_EXCEPTIONS): Remove mips16/micromips entry. OK, thanks. Richard
Re: PING: Re: [patch] implement simd loops in trunk (OMP_SIMD)
On 08/08/13 18:42, Richard Henderson wrote: On 08/08/2013 02:06 AM, Aldy Hernandez wrote: The hash is not really mapping the simd DECL to the simduid, since that's just a matter of DECL_UID(simduid), but the OMP simd array to the index used to reference it (simduid), like thus: _7 = GOMP_SIMD_LANE (simduid.0) ... ... D.1737[_7] = stuff; decl_to_simduid is a mapping from the simduid.0 to the D.1737[] (the OMP simd array). Agreed, or am I missing something? Agreed. Yeah, that's a bit confusing. Ok, I will change the hash name and add some comments.
Re: [C++ Patch / RFC] PR 46206
On 08/08/2013 08:31 PM, Paolo Carlini wrote: On 08/08/2013 08:18 PM, David Edelsohn wrote: Why does the patch and fix have any architecture or OS-dependency? It's tricky, but it depends on the way are internally stored and handled *multiple* TYPE_DECL for essentially the same typedef. The class layout must involve both such typedef and a few data members. ... and at least a virtual function, forgot this important detail. I don't think you can construct a reject-valid of this type on Linux without a virtual function. Paolo.
[C++ RFC / Patch] PR 54080, PR 52875 and more (aka SFINAE vs template recursion depth)
Hi, this is, IMHO, a rather interesting issue. I was working on PR 54080, where currently with ICE pretty badly for Error reporting routines re-entered without producing any sensible error message. Figured out that the core of the issue are the error messages in push_tinst_level about template instantiation depth exceeded. Drafted the attached, the error message is great with no ICE: 54080.C: In instantiation of ‘decltype (footemplateclass T class vector(foo(input, func1), funcrest)) foo(const vectorint, const Func1, FuncRest) [with OutType = vector; Func1 = int; FuncRest = int; decltype (foovector(foo(input, func1), funcrest)) = vectorint]’: 54080.C:25:22: required from here 54080.C:19:2: error: return-statement with no value, in function returning ‘vectorint’ [-fpermissive] return; when I ran the testsuite, I found something (tidied): FAIL: g++.dg/cpp0x/decltype26.C (test for errors, line 6) FAIL: g++.dg/cpp0x/decltype26.C (test for excess errors) FAIL: g++.dg/cpp0x/decltype28.C (test for errors, line 11) FAIL: g++.dg/cpp0x/decltype28.C (test for warnings, line 15) FAIL: g++.dg/cpp0x/decltype29.C (test for errors, line 12) FAIL: g++.dg/cpp0x/decltype29.C (test for excess errors) the really interesting one is decltype28.C, which we don't reject anymore, we simply accept it. What is happening is that the overload which leads to excessive template instantiation depth is SFINAE-ed away and the other one wins! Thus, this is the core of my message: it seems that we behave wrt this issue - SFINAE vs template instantiation depth - in a different way vs current clang++ and icc: we produce hard error messages in SFINAE contexts. Is that intended? I find the issue interesting, arguably a template instantiation depth exceeded isn't just like any other error. With the attached draft we handle quite a few other testcases I have seen around in a different way, for example c++/52875 is automatically fixed (assuming we want to behave like clang++ and icc). Thanks! Paolo. Index: cp-tree.h === --- cp-tree.h (revision 201588) +++ cp-tree.h (working copy) @@ -5541,7 +5541,7 @@ extern tree fold_non_dependent_expr_sfinae(tree, extern bool alias_type_or_template_p(tree); extern bool alias_template_specialization_p (const_tree); extern bool explicit_class_specialization_p (tree); -extern int push_tinst_level (tree); +extern int push_tinst_level (tree, tsubst_flags_t); extern void pop_tinst_level (void); extern struct tinst_level *outermost_tinst_level(void); extern void init_template_processing (void); Index: mangle.c === --- mangle.c(revision 201588) +++ mangle.c(working copy) @@ -3429,7 +3429,7 @@ mangle_decl_string (const tree decl) { struct tinst_level *tl = current_instantiation (); if ((!tl || tl-decl != decl) - push_tinst_level (decl)) + push_tinst_level (decl, tf_warning_or_error)) { template_p = true; saved_fn = current_function_decl; Index: pt.c === --- pt.c(revision 201588) +++ pt.c(working copy) @@ -6995,7 +6995,7 @@ add_pending_template (tree d) level = !current_tinst_level || current_tinst_level-decl != d; if (level) -push_tinst_level (d); +push_tinst_level (d, tf_warning_or_error); pt = ggc_alloc_pending_template (); pt-next = NULL; @@ -8021,24 +8021,28 @@ static GTY(()) struct tinst_level *last_error_tins for diagnostics and to restore it later. */ int -push_tinst_level (tree d) +push_tinst_level (tree d, tsubst_flags_t complain) { struct tinst_level *new_level; if (tinst_depth = max_tinst_depth) { last_error_tinst_level = current_tinst_level; - if (TREE_CODE (d) == TREE_LIST) - error (template instantiation depth exceeds maximum of %d (use - -ftemplate-depth= to increase the maximum) substituting %qS, - max_tinst_depth, d); - else - error (template instantiation depth exceeds maximum of %d (use - -ftemplate-depth= to increase the maximum) instantiating %qD, - max_tinst_depth, d); - print_instantiation_context (); + if (complain tf_error) + { + if (TREE_CODE (d) == TREE_LIST) + error (template instantiation depth exceeds maximum of %d (use + -ftemplate-depth= to increase the maximum) + substituting %qS, max_tinst_depth, d); + else + error (template instantiation depth exceeds maximum of %d (use + -ftemplate-depth= to increase the maximum) + instantiating %qD, max_tinst_depth, d); + print_instantiation_context (); + } + return
patch for correct mode use by LRA for save/restore generation
The following patch implements correct mode use for save/restore generation. The patch was successfully bootstrapped and tested on x86/x86-64, ppc64, s390. The patch also makes some tuning in alternative matching and adds more debugging printing. Committed as rev. 201611. 2013-08-08 Vladimir Makarov vmaka...@redhat.com * lra-constraints.c (emit_spill_move): Remove assert. (process_alt_operands): Add more debugging output. Increase reject for spilling into memory. Decrease reject for reloading scratch. (split_reg): Use HARD_REGNO_CALLER_SAVE_MODE. Index: lra-constraints.c === --- lra-constraints.c (revision 201544) +++ lra-constraints.c (working copy) @@ -859,8 +859,9 @@ { if (GET_MODE (mem_pseudo) != GET_MODE (val)) { - lra_assert (GET_MODE_SIZE (GET_MODE (mem_pseudo)) - = GET_MODE_SIZE (GET_MODE (val))); + /* Usually size of mem_pseudo is greater than val size but in +rare cases it can be less as it can be defined by target +dependent macro HARD_REGNO_CALLER_SAVE_MODE. */ if (! MEM_P (val)) { val = gen_rtx_SUBREG (GET_MODE (mem_pseudo), @@ -1423,8 +1424,14 @@ overall = losers = reject = reload_nregs = reload_sum = 0; for (nop = 0; nop n_operands; nop++) - reject += (curr_static_id - -operand_alternative[nalt * n_operands + nop].reject); + { + int inc = (curr_static_id +-operand_alternative[nalt * n_operands + nop].reject); + if (lra_dump_file != NULL inc != 0) + fprintf (lra_dump_file, +Staticly defined alt reject+=%d\n, inc); + reject += inc; + } early_clobbered_regs_num = 0; for (nop = 0; nop n_operands; nop++) @@ -1598,7 +1605,14 @@ || (find_regno_note (curr_insn, REG_DEAD, REGNO (operand_reg[nop])) == NULL_RTX)) - reject += 2; + { + if (lra_dump_file != NULL) + fprintf + (lra_dump_file, +%d Matching alt: reject+=2\n, +nop); + reject += 2; + } } /* If we have to reload this operand and some previous operand also had to match the same @@ -1854,16 +1868,35 @@ { if (in_hard_reg_set_p (this_costly_alternative_set, mode, hard_regno[nop])) - reject++; + { + if (lra_dump_file != NULL) + fprintf (lra_dump_file, +%d Costly set: reject++\n, +nop); + reject++; + } } else { /* Prefer won reg to spilled pseudo under other equal conditions. */ + if (lra_dump_file != NULL) + fprintf + (lra_dump_file, + %d Non pseudo reload: reject++\n, + nop); reject++; if (in_class_p (operand_reg[nop], this_costly_alternative, NULL)) - reject++; + { + if (lra_dump_file != NULL) + fprintf + (lra_dump_file, + %d Non pseudo costly reload: + reject++\n, + nop); + reject++; + } } /* We simulate the behaviour of old reload here. Although scratches need hard registers and it @@ -1872,7 +1905,13 @@ might cost something but probably less than old reload pass believes. */ if (lra_former_scratch_p (REGNO (operand_reg[nop]))) - reject += LRA_LOSER_COST_FACTOR; + { + if (lra_dump_file != NULL) + fprintf (lra_dump_file, +%d Scratch win: reject+=3\n, +nop); + reject += 3; + } } } else if (did_match) @@ -1914,7 +1953,12 @@
[patch][PR/42955] Don't install $(target)/bin/gcc, gfortran, etc.
As discussed in PR/42955, when GCC is built as a cross-compiler, it will install gcc, g++, c++, and gfortran binaries in $(target)/bin, as well as installing the $target-gcc and so forth in bin. However, these binaries in $(target)/bin do not work; they cannot find libexec. More to the point, this bug has been open for three years with no traffic, and the failure started significantly before that. Clearly, making these work is not a priority. Further, these binaries are real files, not symlinks or hard links; they take up actual space. As discussed on the bug, Joseph argues that $(target)/bin contains executables from binutils for internal use by GCC; that's its sole purpose. The files installed by GCC there aren't used by GCC (rather, the public installed copy of the driver gets used when collect2 needs to call back to the driver), so shouldn't be installed. Thus, this patch, which simply removes these broken executables. Tested by building a cross-compiler and confirming that they are gone, and by building a native compiler and confirming that the expected bin/gcc, bin/g++, bin/c++, and bin/gfortran are still present. Ok to commit? - Brooks 2013-08-08 Brooks Moses bmo...@google.com PR driver/42955 * Makefile.in: Do not install driver binaries in $(target)/bin. PR driver/42955 * Make-lang.in: Do not install driver binaries in $(target)/bin. 2013-08-08_remove-target-gcc-bins.diff Description: Binary data
[PATCH, vtv update] Add documentation for --enable-vtable-verify configure option...
The attached patch adds documentation for the --enable-vtable-verify configure option to install.texi. Is this ok to commit? -- Caroline TIce cmt...@google.com 2013-08-08 Caroline Tice cmt...@google.com * doc/install.texi: Add documentation for the --enable-vtable-verify configure option. vtv-update-doc.patch Description: Binary data
[PATCH, vtv update] Fix /tmp directory issues in libvtv
This patch changes where the logging file mechanism in libvtv tries to write its log files. Instead of trying to use /tmp, it now first looks for an environment variable VTV_LOGS_DIR. If it can't find that it looks for the environment variable HOME. If it can't find that either, it uses the current directory. This also adds O_NOFOLLOW to the open command. There was a request that we also use O_EXCL, but that would cause a problem because we occasionally want to be able to append to existing log files, and adding O_EXCL would cause that to fail. I also added the uid and pid to the log file names. Is this patch OK to commit? -- Caroline Tice cmt...@google.com 2013-08-08 Caroline Tice cmt...@google.com * vtv_utils.cc : Include stdlib.h (log_dirs): Remove file static constant. (__vtv_open_log): Increase size of log file name. Add the user and process ids to the file name. Do not put the log files in /tmp. Instead try to get the directory name from environment variables; if that fails use the current directory. Add O_NOFOLLOW to the flags for 'open'. Update function comment. vtv-update-tmpdir.patch Description: Binary data
Re: [PATCH, vtv update] Add documentation for --enable-vtable-verify configure option...
On Thu, 8 Aug 2013, Caroline Tice wrote: The attached patch adds documentation for the --enable-vtable-verify configure option to install.texi. Is this ok to commit? Could you please answer the questions I raised in the first four paragraphs of http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00427.html at greater length? This patch appears to document it just as a libstdc++ configure option, but as noted there it appears to affect more than libstdc++ (even though it would be more sensible if it *only* affected libstdc++, and even better if it just enabled extra multilibs). -- Joseph S. Myers jos...@codesourcery.com
[PATCH, vtv update] Change fixed size array to a vector; fix diagnostic messages.
This patch replaces the fixed sized array that was holding vtable pointers for a particular class hierarchy with a vector, allowing for dynamic resizing. It also fixes issues with the warning diagnostics. I am in the process of running regression tests with this patch; assuming they all pass, is this patch OK to commit? -- Caroline Tice cmt...@google.com 2013-08-08 Caroline Tice cmt...@google.com * vtable-class-hierarchy.c: Remove unnecessary include statements. (MAX_SET_SIZE): Remove unnecessary constant. (register_construction_vtables): Make vtable_ptr_array parameter into a vector; remove num_args parameter. Change array accesses to vector accesses. (register_other_binfo_vtables): Ditto. (insert_call_to_register_set): Ditto. (insert_call_to_register_pair): Ditto. (output_set_info): Ditto. Also change warning calls to warning_at calls, and fix format of warning messages. (register_all_pairs): Change vtbl_ptr_array from an array into a vector. Remove num_vtable_args (replace with calls to vector length). Change array stores accesses to vector functions. Change calls to register_construction_vtables, register_other_binfo_vtables, insert_call_to_register_set, insert_call_to_register_pair and output_set_info to match their new signatures. Change warning to warning_at and fix the format of the warning message. vtv-update.patch Description: Binary data vtv-update.patch Description: Binary data
Re: [PATCH, vtv update] Fix /tmp directory issues in libvtv
On Aug 8, 2013, at 3:09 PM, Caroline Tice cmt...@google.com wrote: This patch changes where the logging file mechanism in libvtv tries to write its log files. Instead of trying to use /tmp, it now first looks for an environment variable VTV_LOGS_DIR. If it can't find that it looks for the environment variable HOME. Hum… I kinda don't think HOME should be used.
Re: [FIXED] Generic lambda symbol table bug
On 07.08.2013 20:56, Adam Butcher wrote: On 07.08.2013 16:59, Jason Merrill wrote: On 08/07/2013 03:52 AM, Adam Butcher wrote: But a cleaner way might be to extend the processing template declaration state from lambda declarator all the way to the end of the lambda body. This would match with the scenario that occurs with a standard in-class member function template definition. To do that elegantly would require a bit of refactoring of the lambda parser code. It isn't already set through the whole lambda body? No. It is within cp_parser_lambda_declarator_opt. That seems necessary to support non-trivial lambda bodies; otherwise we won't be able to handle dependence properly. Agreed. Okay, I will produce three patches: 1) Refactor existing monomorphic implementation to give a cleaner parser separation of concerns; extract the fco creation and provide begin and end for the lambda function operator. I did this---well not exactly as written---but I extended the template decl state through until the end of the lambda body. And it works with my dependence test. However, the previous version does too. The following program works fine (conv ops an all) with the refactored version and without it. I'm now questioning whether it is worth making any change in this area. What do you think? The test code is as follows and the resulting program correctly executes S::N::test() 4 times as expected -- were you thinking of some other dependence case? #include iostream struct S { struct N { float test () { std::cout S::N::test() - 7.f\n; return 7.f; } }; }; int main() { auto f = [] typename T (T const s) { typename T::N x; return x.test (); }; auto g = [] (auto const s) { typename std::decaydecltype (s)::type::N x; return x.test (); }; S i; f(i); g(i); float (*pfn) (S const) = f; pfn(i); pfn = g; return pfn(i); } Error messages are reasonable also. Omitting the necessary 'typename's gives: : need ‘typename’ before ‘T::N’ because ‘T’ is a dependent scope : need ‘typename’ before ‘typename std::decaydecltype (s)::type::N’ because ‘typename std::decaydecltype (s)::type’ is a dependent scope Passing a local 'struct X {}' instead of 'S' gives: : In instantiation of ‘auto main()::lambda(const T) const [with T = main()::X]’: :24:9: required from here :14:23: error: no type named ‘N’ in ‘struct main()::X’ ... So all seems to be okay with both versions. Any ideas why? Cheers, Adam
Re: [PATCH/Merge Request] Vtable Verification feature.
On Wed, Aug 7, 2013 at 6:03 PM, Joseph S. Myers jos...@codesourcery.com wrote: Looking at the patch as committed, there seems to be some confusion about the nature of the --enable-vtable-verify configure option. Yes, there is a bit. It's documented only for libstdc++. Now, I still consider the existence of separate documentation for libstdc++ configure options to be unfortunate - I think all configure options for GCC should be documented in one place - but that's a separate matter. Although only documented for libstdc++, it actually appears to be used in the libgcc and libvtv configure scripts. The original intent of this flag was that if --enable-vtable-verify was NOT used, then NOTHING having to do with vtable verification would be built anywhere in the compiler, i.e. the binaries, libraries, etc. would not contain anything that wasn't there before the vtable verification patch was committed. The idea (and implementation) has evolved a bit, and I am not sure what the right way to handle this option is at the moment. Given that it has effects on more than just libstdc++, it needs documenting in gcc/doc/install.texi, the main documentation of configure options for GCC. I recently submitted a patch that adds the documentation for my current understanding of the way this option works. Then there's the question of what the semantics should be. My presumption is that the feature should be available for all GCC builds, at least by default on platforms where it can work, and the only thing needing a configure option should be whether the checks are enabled for libstdc++ itself (and ideally that would work by multilibbing libstdc++ rather than needing separate GCC builds to get libstdc++ with and without the checks). Actually if you ever want to use the feature with your compiler you should build your compiler with --enable-vtable-verify. This will, as you noted, insert calls in libstdc++ to build the verification data structures and to verify the virtual calls in libstdc++ itself. However libstdc++ itself contains 'stub' (do-nothing) versions of the functions that build the data structures and perform the verification. So if you want to turn on verification with libstdc++, you link it with libvtv (which contains the real versions of those functions) and if you don't want verification with libstdc++ you just don't link in libvtv. There is no need to multiple versions of libstdc++. Thus if the platform supports the feature, all relevant libgcc files should be built, and anything for libstdc++ needed for user code to use the feature should be built - the only thing not enabled by default would be checks for libstdc++'s classes' own vtables. (And unless there are difficulties in building the libgcc files on some systems, they could be built unconditionally, whether or not any other support needed for libvtv is present. I supposed the libgcc files could be built all the time (on systems that support libvtv). Would there be any down side to this? Actually, it looks like they may depend on an ELF target, but not on anything more.) Could you confirm that the libstdc++ ABI is not affected by the configure option - that the same symbols, at the same symbol versions, with the same semantics, are exported from libstdc++.so for builds with and without the feature enabled? The libstdc++ ABI has been enhanced to export the vtable verification functions for which it contains stub versions (otherwise they could never be overwritten by the versions in libvtv). Other than that the libstdc++ ABI exports the same symbols at the same symbol versions with the same semantics. I believe this export is unconditional. The file cp/vtable-class-hierarchy.c includes tm.h. Includes of tm.h from front ends are discouraged, and should have comments on them listing what target macros are used by the file in question (and so need to be converted to hooks before the include can be removed). Could you add such a comment to the #include (or if it's redundant, remove all redundant #includes from that file)? You have a +#define MAX_SET_SIZE 5000 which superficially looks like an arbitrary limit. Could you add a comment explaining why no input files, not matter how extreme, could ever exceed the limit of 5000? You have a couple of gcc_asserts regarding this limit, and an on-stack array for which it's at least not immediately obvious that all accesses are checked to ensure that a buffer overrun is impossible. If you have an arbitrary limit, and some input *can* exceed it (so triggering the gcc_asserts or buffer overrun), the right fix is of course to remove it, probably using vec.h to produce a dynamically growing array instead of hardcoding a size at all. But failing that, exceeding the limit must result in a sorry () call, not an assertion failure or buffer overrun, neither of which is acceptable behavior for any compiler input whatever. Other people have
Re: [PATCH, vtv update] Fix /tmp directory issues in libvtv
On Thu, Aug 8, 2013 at 3:26 PM, Mike Stump mikest...@comcast.net wrote: On Aug 8, 2013, at 3:09 PM, Caroline Tice cmt...@google.com wrote: This patch changes where the logging file mechanism in libvtv tries to write its log files. Instead of trying to use /tmp, it now first looks for an environment variable VTV_LOGS_DIR. If it can't find that it looks for the environment variable HOME. Hum… I kinda don't think HOME should be used. Why not?
Fix PHI IDs in LTO streaming
Hi, LTO streaming renumbers gimple statements. It however forgets about PHIs that causes duplicated gimple ids and later fun everywhere (especially for me when I want to use them for reference streaming). This patch assign numbers to SSA names too. Since virtual PHIs are not stored, we have to ignore them on stream-out time. I do not think it is cool that they get duplicated ids so I just renumber them afterwards. Bootstrapped/regtested x86_64-linux, comitted as obvious. Honza * lto-streamer-out.c (output_function): Renumber PHIs. * lto-streamer-in.c (input_function): Likewise. Index: lto-streamer-out.c === --- lto-streamer-out.c (revision 201568) +++ lto-streamer-out.c (working copy) @@ -1792,12 +1792,32 @@ output_function (struct cgraph_node *nod FOR_ALL_BB (bb) { gimple_stmt_iterator gsi; + for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (gsi)) + { + gimple stmt = gsi_stmt (gsi); + + /* Virtual PHIs are not going to be streamed. */ + if (!virtual_operand_p (gimple_phi_result (stmt))) + gimple_set_uid (stmt, inc_gimple_stmt_max_uid (cfun)); + } for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (gsi)) { gimple stmt = gsi_stmt (gsi); gimple_set_uid (stmt, inc_gimple_stmt_max_uid (cfun)); } } + /* To avoid keeping duplicate gimple IDs in the statements, renumber +virtual phis now. */ + FOR_ALL_BB (bb) + { + gimple_stmt_iterator gsi; + for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (gsi)) + { + gimple stmt = gsi_stmt (gsi); + if (virtual_operand_p (gimple_phi_result (stmt))) + gimple_set_uid (stmt, inc_gimple_stmt_max_uid (cfun)); + } + } /* Output the code for the function. */ FOR_ALL_BB_FN (bb, fn) Index: lto-streamer-in.c === --- lto-streamer-in.c (revision 201568) +++ lto-streamer-in.c (working copy) @@ -908,6 +908,11 @@ input_function (tree fn_decl, struct dat FOR_ALL_BB (bb) { gimple_stmt_iterator gsi; + for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi); gsi_next (gsi)) + { + gimple stmt = gsi_stmt (gsi); + gimple_set_uid (stmt, inc_gimple_stmt_max_uid (cfun)); + } for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (gsi)) { gimple stmt = gsi_stmt (gsi); @@ -917,7 +922,14 @@ input_function (tree fn_decl, struct dat stmts = (gimple *) xcalloc (gimple_stmt_max_uid (fn), sizeof (gimple)); FOR_ALL_BB (bb) { - gimple_stmt_iterator bsi = gsi_start_bb (bb); + gimple_stmt_iterator bsi = gsi_start_phis (bb); + while (!gsi_end_p (bsi)) + { + gimple stmt = gsi_stmt (bsi); + gsi_next (bsi); + stmts[gimple_uid (stmt)] = stmt; + } + bsi = gsi_start_bb (bb); while (!gsi_end_p (bsi)) { gimple stmt = gsi_stmt (bsi);
Re: [PATCH/Merge Request] Vtable Verification feature.
On Thu, 8 Aug 2013, Caroline Tice wrote: Actually if you ever want to use the feature with your compiler you should build your compiler with --enable-vtable-verify. This will, as you noted, insert calls in libstdc++ to build the verification data structures and to verify the virtual calls in libstdc++ itself. However libstdc++ itself contains 'stub' (do-nothing) versions of the functions that build the data structures and perform the verification. So if you want to turn on verification with libstdc++, you link it with libvtv (which contains the real versions of those functions) and if you don't want verification with libstdc++ you just don't link in libvtv. There is no need to multiple versions of libstdc++. But presumably libstdc++ with the extra calls is less efficient than libstdc++ without them, even if the calls are just to stub functions? A GNU/Linux distributor would likely want to enable their users to use this functionality, meaning distributing a GCC build with libvtv included and a version of libstdc++ with the calls present for users who wish to build programs (linking with libstdc++ and libvtv) for which the calls in libstdc++ are checked. But they might not want to have even the stub calls executed for all installed C++ programs. So it would seem natural to provide both copies of libstdc++ - and desirable for this to be possible without needing separate GCC builds. I supposed the libgcc files could be built all the time (on systems that support libvtv). Would there be any down side to this? In my model, it's appropriate to build those independent of the libstdc++ configure option - and likewise to build everything in libvtv independent of the configure option. There should be no need for the configure option to affect anything other than, at most, libstdc++ - that is, the handling of the option in other configure scripts should be removed, with the case where the relevant support is built being enabled by default. For certain use cases the current working directory is not our preferred place to put the log files. I have recently submitted a patch that tries to use environment variables to determine where to put the log files. It first checks to see if the user has defined VTV_LOGS_DIR, in which case it will use that. If that fails, it tries to find and use HOME. If that also fails, it falls back on using the working directory. I hope that is ok? I have modified the call to open to take O_NOFOLLOW, but O_EXCL will do the wrong thing, as we sometimes wish to append to the log files and O_EXCL fails if you attempt to open an existing file (according to the documentation I read). The list of directories seems plausible (better than using /tmp, anyway). Classically O_EXCL was needed in such cases (if the log might end up getting written to a file in a directory also writable by an attacker) not just because of symlink attacks but also because it was possible to create a hard link to another user's file, with similar potential for attacks as with symlinks. Nowadays systems are more likely to restrict such hard link creation (but they are also likely to have similar restrictions to make symlink attacks harder as well). -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Sanitize block partitioning under -freorder-blocks-and-partition
On Thu, Aug 8, 2013 at 3:23 PM, Jan Hubicka hubi...@ucw.cz wrote: Hi, Martin Liska was kind enough to generate disk seeking graph of gimp statrup with his function reordering. His code simply measures time of firest execution of a function and orders functions in the given order. The functions stay in the subsections (unlikely/startup/exit/hot/normal) that are then glued together in this order. I am attaching disk seeking with and without -freorder-blocks-and-partition (with your patch). In 2.pdf you can see two increasing sequences in the text segment. If I am not mistaken the bottom one comes for hot and the top one for normal section. The big unused part on bottom is unlikely section since most of gimp is not trained. 2.pdf is reordered with Martin's technique? Now 1.pdf is with -freorder-blocks-and-partition and your patch. You can see there is third sequence near bottom of the text seciton. that is beggining of unlikely section, so it tracks jumps where we fall into cold section of function. 1.pdf is generated using the usual FDO + -freorder-blocks-and-partition (i.e. not using Martin's technique)? It still seems rather bad (i.e. good part of unlikely section is actually used). I think the dominator based approach is not going to work too reliably (I can fix my testcase to contain multiple nested conditionals and then the heuristic about predecestors won't help). Yes, this doesn't look good. Did you use the latest version of my patch that doesn't walk the dominators? Do you know how many training runs are done for this benchmark? I think a lot of the issues that you pointed out with the hot loop preceded by non-looping conditional code as in your earlier example, or multiple nested conditionals, comes from the fact that the cold cutoff is not 0, but some number less than the number of training runs. Perhaps the cutoff for splitting should be 0. Then the main issue that needs to be corrected is profile insanities, not code that is executed once (since that would not be marked cold). The only other issue that I can think of here is that the training data was not representative and didn't execute these blocks. What about simply walking the CFG from entry through all edges with non-0 counts and making all reachable blocks hot + forcingly make any hot blocks not reachable this way reachable? Is this different than what I currently have + changing the cold cutoff to 0? In that case any blocks reachable through non-0 edges should be non-0 and marked hot, and the current patch forces the most frequent paths to all hot blocks to be hot. Thanks, Teresa I think we are really looking primarily for dead parts of the functions (sanity checks/error handling) that should not be visited by train run. We can then see how to make the heuristic more aggressive? Honza -- Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
Re: [PATCH] Fix PR48493
In the below, the test case tries to write to the stack outside the bounds of the s variable? I can't imagine any good coming from this, and indeed, would be nice for the compiler to complain about such code. If S had a few more bytes at the end, at least the code would not be wildly bad. Thoughts? On May 31, 2012, at 3:59 AM, Richard Guenther rguent...@suse.de wrote: This fixes PR48493 by backporting a one-liner - we should not go the movmisalign path for destinations that are not memory. Bootstrapped and tested on x86_64-unknown-linux-gnu and on mips by Andrew, installed. Richard. 2012-05-31 Richard Guenther rguent...@suse.de PR middle-end/48493 * expr.c (expand_assignment): Do not use movmisalign on non-memory. * gcc.dg/torture/pr48493.c: New testcase. Index: gcc/testsuite/gcc.dg/torture/pr48493.c === *** gcc/testsuite/gcc.dg/torture/pr48493.c(revision 0) --- gcc/testsuite/gcc.dg/torture/pr48493.c(revision 0) *** *** 0 --- 1,18 + /* { dg-do compile } */ + + typedef long long T __attribute__((may_alias, aligned (1))); + + struct S + { + _Complex float d __attribute__((aligned (8))); + }; + + void bar (struct S); + + void + f1 (T x) + { + struct S s; + *(T *) ((char *) s.d + 1) = x; + bar (s); + }
Re: [PATCH/Merge Request] Vtable Verification feature.
On 08/08/2013 06:34 PM, Caroline Tice wrote: Actually if you ever want to use the feature with your compiler you should build your compiler with --enable-vtable-verify. This will, as you noted, insert calls in libstdc++ to build the verification data structures and to verify the virtual calls in libstdc++ itself. However libstdc++ itself contains 'stub' (do-nothing) versions of the functions that build the data structures and perform the verification. How do you ensure that the libvtv versions get used rather than the stubs in libstdc++? Jason
Re: [C++ RFC / Patch] PR 54080, PR 52875 and more (aka SFINAE vs template recursion depth)
On 08/08/2013 03:54 PM, Paolo Carlini wrote: the really interesting one is decltype28.C, which we don't reject anymore, we simply accept it. What is happening is that the overload which leads to excessive template instantiation depth is SFINAE-ed away and the other one wins! Thus, this is the core of my message: it seems that we behave wrt this issue - SFINAE vs template instantiation depth - in a different way vs current clang++ and icc: we produce hard error messages in SFINAE contexts. Is that intended? Yes, that is intended. Changing that could mean that the meaning of code depends on what max depth the user selected. Jason
Re: [PATCH] Fix expansion issues on type changing MEM_REFs on LHS (PR middle-end/48335)
On Mar 30, 2011, at 9:05 AM, Jakub Jelinek ja...@redhat.com wrote: MEM_REFs which can represent type punning on lhs don't force non-gimple types to be addressable. This causes various problems in the expander, which wasn't prepared to handle that. This patch tries to fix what I've found and adds a bunch of testcases. The original report was with just -O2 on some large testcase from Eigen, most of the testcases have -fno-tree-sra just because I've given up on delta when it stopped reducing at 32KB. The first problem (the one from Eigen) is _mm_store_pd into a std::complexdouble var, which is a single field and thus has DCmode TYPE_MODE. As starting with 4.6 that var is not TREE_ADDRESSABLE, its DECL_RTL is a CONCAT, and for assignments to concat expand_assignment was expecting either that from has COMPLEX_TYPE (and matching mode to the store), or that it is a real or imaginary subpart store, thus when trying to store a V2DF mode value it expected it to be real part store (bitpos == 0) and tried to set a DFmode pseudo from V2DFmode rtx. Further testing revealed that it is possible to hit many other cases with CONCAT destination, it can be a store of just a few bits, or can overlap parts of both real and imaginary, or be partially out of bounds. The patch handles the case from Eigen - bitpos == 0 bitsize == GET_MODE_BITSIZE (GET_MODE (to_rtx)) non-COMPLEX_TYPE by setting each half separately, if it is a store which is not touching one of the parts by just adjusting bitpos for the imaginary case and storing just to one of the parts (this is the bitpos + bitsize half_bitsize resp. bitpos = half_bitsize case) and finally adds a generic slow one for the very unusual cases with partial overlap of both. After testing it with the testcases I wrote, I found a bunch of other ICEs though, and reproduced them even without CONCAT on the LHS (the testcases below which don't contain any _Complex keyword). --- gcc/testsuite/gcc.dg/pr48335-2.c.jj 2011-03-30 10:57:29.0 +0200 +++ gcc/testsuite/gcc.dg/pr48335-2.c 2011-03-29 18:27:53.0 +0200 @@ -0,0 +1,58 @@ +/* PR middle-end/48335 */ +/* { dg-do compile } */ +/* { dg-options -O2 -fno-tree-sra } */ + +typedef long long T __attribute__((may_alias, aligned (1))); +typedef short U __attribute__((may_alias, aligned (1))); + +struct S +{ + _Complex float d __attribute__((aligned (8))); +}; + +void bar (struct S); + +void +f1 (T x) +{ + struct S s; + *(T *) ((char *) s.d + 1) = x; + __real__ s.d *= 7.0; + bar (s); +} + +void +f2 (int x) +{ + struct S s = { .d = 0.0f }; + ((U *)((char *) s.d + 1))[0] = x; + __real__ s.d *= 7.0; + bar (s); +} + +void +f3 (int x) +{ + struct S s = { .d = 0.0f }; + ((U *)((char *) s.d + 1))[1] = x; + __real__ s.d *= 7.0; + bar (s); +} + +void +f4 (int x) +{ + struct S s = { .d = 0.0f }; + ((U *)((char *) s.d + 1))[2] = x; + __real__ s.d *= 7.0; + bar (s); +} + +void +f5 (int x) +{ + struct S s = { .d = 0.0f }; + ((U *)((char *) s.d + 1))[3] = x; + __real__ s.d *= 7.0; + bar (s); +} So, this test case writes outside the bounds of s. I think it is a nice test case for a missed diagnostic of some form, but for code that is supposed to compile, kinda hate it. Thoughts?
Re: [PATCH/Merge Request] Vtable Verification feature.
Which version gets used depends on their ordering in the link line. When -fvtable-verify=std or -fvtable-verify=preinit is used, the gcc driver inserts -lvtv very early into the link line (earlier than -lstdc++), so this happens automatically. -- Caroline cmt...@google.com On Thu, Aug 8, 2013 at 4:33 PM, Jason Merrill ja...@redhat.com wrote: On 08/08/2013 06:34 PM, Caroline Tice wrote: Actually if you ever want to use the feature with your compiler you should build your compiler with --enable-vtable-verify. This will, as you noted, insert calls in libstdc++ to build the verification data structures and to verify the virtual calls in libstdc++ itself. However libstdc++ itself contains 'stub' (do-nothing) versions of the functions that build the data structures and perform the verification. How do you ensure that the libvtv versions get used rather than the stubs in libstdc++? Jason
Re: [PATCH 3/5] Atomic type qualifier - front end changes
Observations on this patch: * build_qualified_type sets the qualifiers to exactly the set specified. Thus, it looks like your handle_atomic_attribute will remove existing qualifiers when adding atomic. * c-aux-info.c is meant to be generating actual valid C declarations, I think, meaning _Atomic rather than atomic. * The code you have checking for _Atomic with function declarators appears to be in the wrong place. What you're testing is the combination of (_Atomic appears in declaration specifiers, as given by atomicp) and (some declarator in the nested sequence of declarators is a function declarator). But there are valid cases for this - for example a function returning a pointer to atomic. And there are invalid cases involving an atomic-qualified function type that you don't catch there - for example, if _Atomic is applied to a typedef for a function type rather than together with the declarator. The relevant issue is whether the *function type* itself gets qualified. There are already pedwarns-if-pedantic for this, given that there's undefined behavior in ISO C for qualifiers on function types in general, but making this case a hard error seems reasonable enough. To do that, you need to adjust the cases case cdk_pointer: { /* Merge any constancy or volatility into the target type for the pointer. */ if (pedantic TREE_CODE (type) == FUNCTION_TYPE type_quals) pedwarn (loc, OPT_Wpedantic, ISO C forbids qualified function types); and similar for typedefs, type names, parameters and actual function declarations, to give such errors. And similarly, for all the various cases of things that can be declared, whenever qualifiers get applied in grokdeclarator you need to ensure an error is given if the type is an array type, since that is also a constraint violation in ISO C. * The pedwarn for _Atomic outside C11 mode should I think only be a pedwarn-if-pedantic, so pass OPT_Wpedantic instead of 0. And the text should match other such pedwarns (i.e. ISO C90 does not support _Atomic or ISO C99 does not support _Atomic, depending on the standard version selected). * The parser comments with syntax should refer to _Atomic not atomic. * You change convert_for_assignment regarding discarding qualifiers. Doing so is correct as far as it goes - not because of the reason in your comment, but because in C11 terms _Atomic isn't a type qualifier. But I don't think the change is enough. Because it's inside a conditional checking for allowed cases: either side being a pointer to a void type, or the target types being compatible. Now the rule in 6.5.16.1#1 refers to qualified or unqualified version of void, which in ISO C terms does not include _Atomic void, so those checks need to disallow _Atomic void. And similarly, it's not enough for comp_target_types to match the unqualified types when it also needs to match whether _Atomic is specified (for that, comp_target_types should probably be changed - conditional expressions have the same issue). The above relate specifically to what's in the patch. The listed issues should of course have testcases added. I still need to review it on the basis of reviews of C11 for references to qualifiers or _Atomic, to see what might be missing from the patch, and on the basis of reviews of the C front end for handling of qualifiers, to see what places might need to handle _Atomic specially; I'll do those reviews separately. As previously noted, one thing missing is the _Atomic ( type-name ) syntax. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH] Fix expansion issues on type changing MEM_REFs on LHS (PR middle-end/48335)
On Mar 30, 2011, at 9:05 AM, Jakub Jelinek ja...@redhat.com wrote: + else if (bitpos = mode_bitsize / 2) + result = store_field (XEXP (to_rtx, 1), bitsize, + bitpos - mode_bitsize / 2, mode1, from, + TREE_TYPE (tem), get_alias_set (to), + nontemporal); - gcc_assert (bitpos == 0 || bitpos == GET_MODE_BITSIZE (mode1)); - result = store_expr (from, XEXP (to_rtx, bitpos != 0), false, -nontemporal); + rtx temp = assign_stack_temp (GET_MODE (to_rtx), + GET_MODE_SIZE (GET_MODE (to_rtx)), + 0); + write_complex_part (temp, XEXP (to_rtx, 0), false); + write_complex_part (temp, XEXP (to_rtx, 1), true); + result = store_field (temp, bitsize, bitpos, mode1, from, + TREE_TYPE (tem), get_alias_set (to), + nontemporal); + emit_move_insn (XEXP (to_rtx, 0), read_complex_part (temp, false)); + emit_move_insn (XEXP (to_rtx, 1), read_complex_part (temp, true)); I think this is bad. I think this patch fixes it. The problem is that we can't write outside the bounds of the object. So, instead we punt out of the register case, and instead spill it to memory that _is_ big enough, and then spill it back to registers. Of course, I'd rather emit a diagnostic when extra is non-zero… but not sure people yet buy into that around here. Thoughts? diff --git a/gcc/expr.c b/gcc/expr.c index 923f59b..f5744b0 100644 --- a/gcc/expr.c +++ b/gcc/expr.c @@ -4815,7 +4815,8 @@ expand_assignment (tree to, tree from, bool nontemporal) bitregion_start, bitregion_end, mode1, from, get_alias_set (to), nontemporal); - else if (bitpos = mode_bitsize / 2) + else if (bitpos = mode_bitsize / 2 + bitpos+bitsize = mode_bitsize) result = store_field (XEXP (to_rtx, 1), bitsize, bitpos - mode_bitsize / 2, bitregion_start, bitregion_end, @@ -4834,8 +4835,12 @@ expand_assignment (tree to, tree from, bool nontemporal) } else { + HOST_WIDE_INT extra = 0; + if (bitpos+bitsize mode_bitsize) + extra = bitpos+bitsize - mode_bitsize; rtx temp = assign_stack_temp (GET_MODE (to_rtx), - GET_MODE_SIZE (GET_MODE (to_rtx))); + GET_MODE_SIZE (GET_MODE (to_rtx)) + + extra); write_complex_part (temp, XEXP (to_rtx, 0), false); write_complex_part (temp, XEXP (to_rtx, 1), true); result = store_field (temp, bitsize, bitpos,
Re: [C++ Patch / RFC] PR 46206
On 08/08/2013 02:31 PM, Paolo Carlini wrote: On 08/08/2013 08:18 PM, David Edelsohn wrote: Why does the patch and fix have any architecture or OS-dependency? Probably the same reason that subtle changes in the testcase changed whether the bug appeared on x86_64-linux. I guess we should figure that out instead of just saying hunh, that's odd. Jason
Re: [vtv] fix default configure
More patches to fix disable issus on non-linux. Here's a patch for the build failure on darwin. Will check in when testing completes. tested x86/linux tested x86_64/darwin12 -benjamin2013-08-08 Benjamin Kosnik b...@rehat.com Michael Meissner meiss...@linux.vnet.ibm.com * configure.tgt : Simplify, just use VTV_SUPPORTED. diff --git a/libvtv/configure.tgt b/libvtv/configure.tgt index a84ed27..801d2f0 100644 --- a/libvtv/configure.tgt +++ b/libvtv/configure.tgt @@ -19,6 +19,7 @@ # lets us skip running autoconf when modifying target specific information. # Filter out unsupported systems. +VTV_SUPPORTED=no case ${target} in x86_64-*-linux* | i?86-*-linux*) VTV_SUPPORTED=yes @@ -30,9 +31,7 @@ case ${target} in arm*-*-linux*) ;; x86_64-*-darwin[1]* | i?86-*-darwin[1]*) - VTV_SUPPORTED=no ;; *) - UNSUPPORTED=1 ;; esac 2013-08-02 Benjamin Kosnik b...@redhat.com * configure.ac: Adjust to check VTV_SUPPORTED. * configure: Regenerated. diff --git a/configure.ac b/configure.ac index bcbc95c..6f3d801 100644 --- a/configure.ac +++ b/configure.ac @@ -561,7 +561,7 @@ if test -d ${srcdir}/libvtv; then AC_MSG_CHECKING([for libvtv support]) if (srcdir=${srcdir}/libvtv; \ . ${srcdir}/configure.tgt; \ - test -n $UNSUPPORTED) + test $VTV_SUPPORTED != yes) then AC_MSG_RESULT([no]) noconfigdirs=$noconfigdirs target-libvtv
Re: [PATCH, vtv update] Fix /tmp directory issues in libvtv
On Aug 8, 2013, at 3:34 PM, Caroline Tice cmt...@google.com wrote: On Thu, Aug 8, 2013 at 3:26 PM, Mike Stump mikest...@comcast.net wrote: On Aug 8, 2013, at 3:09 PM, Caroline Tice cmt...@google.com wrote: This patch changes where the logging file mechanism in libvtv tries to write its log files. Instead of trying to use /tmp, it now first looks for an environment variable VTV_LOGS_DIR. If it can't find that it looks for the environment variable HOME. Hum… I kinda don't think HOME should be used. Why not? Cause it violates a rather nice standard that spans decades for no compelling reason. http://en.wikipedia.org/wiki/Home_directory In the longer run, if linux/posix standardizes per-program logs for the user, then it can be squirted out there. On darwin for example, it would be ~/Library/Logs/argv[0]. These sorts of locations can feature things like, automated reporting, cleaning, compressing, rotation and so on… $HOME does not. Also, the user might have his files, with those names, and you would not want to just stomp on them.
Re: [C++ Patch / RFC] PR 46206
On Thu, Aug 8, 2013 at 8:24 PM, Jason Merrill ja...@redhat.com wrote: On 08/08/2013 02:31 PM, Paolo Carlini wrote: On 08/08/2013 08:18 PM, David Edelsohn wrote: Why does the patch and fix have any architecture or OS-dependency? Probably the same reason that subtle changes in the testcase changed whether the bug appeared on x86_64-linux. I guess we should figure that out instead of just saying hunh, that's odd. There's an AIX system in the GCC Compile Farm and I also can provide additional dump file output if you tell me what would be helpful. Thanks, David
Re: [FIXED] Generic lambda symbol table bug
On 08/08/2013 06:28 PM, Adam Butcher wrote: So all seems to be okay with both versions. Any ideas why? Hmm, it sounds like processing_template_decl is being set after all, even without your change. Jason
Re: [Patch] Whole regex refactoring and current status
On Fri, Aug 9, 2013 at 12:51 AM, Tim Shen timshe...@gmail.com wrote: So here's the change. It's under testing now, but could took several hours. If someone has a faster machine, please tell the result :) Unfortuantely using `unsigned int = unsigned short` cannot work. Instead, I create a enum and do some operator overloadings. This patch passed bootstrap and all tests under the configuration under x86_64: --with-arch=corei7 --with-cpu=corei7 --prefix=/usr/local --enable-clocale=gnu --with-system-zlib --enable-shared --with-demangler-in-ld --with-fpmath=sse --enable-languages=c++ -- Tim Shen a.patch Description: Binary data