patch: don't issue -Wreorder warnings when order doesn't matter
Hello! In my humble opinion the -Wreorder has noise. When the order doesn't matter I would prefer that warnings are not issued. In this email I include a patch that I would like to get comments about. The patch will suppress warnings if all members are initialized with constant values. I am not very good at GCC internals so I wonder if I made some serious mistake when using TREE_VALUE, TREE_CODE, etc? Perhaps I overlook something? Here is sample code that currently generates warnings but my patch suppress those warnings: class Fred { private: int a; int b; public: Fred() : b(0), a(0) { } }; I think the next step will be to suppress the warning if both the members that the message is about are initialized with constant values. Best regards, Daniel Index: gcc/cp/init.c === --- gcc/cp/init.c (revision 176862) +++ gcc/cp/init.c (working copy) @@ -711,6 +711,7 @@ VEC(tree,gc) *vbases; int i; int uses_unions_p; + int all_inits_are_const; /* all members are initialized with a constant value */ /* Build up a list of initializations. The TREE_PURPOSE of entry will be the subobject (a FIELD_DECL or BINFO) to initialize. The @@ -741,6 +742,25 @@ without issuing a warning. */ next_subobject = sorted_inits; + all_inits_are_const = 1; + if (warn_reorder) +{ + for (init = mem_inits; init; init = TREE_CHAIN (init)) +{ + tree tree_value; + + tree_value = TREE_VALUE(init); + if (TREE_CODE(tree_value) == TREE_LIST) +tree_value = TREE_VALUE(tree_value); + + if (TREE_CODE(tree_value) != INTEGER_CST) +{ + all_inits_are_const = 0; + break; +} +} +} + /* Go through the explicit initializers, filling in TREE_PURPOSE in the SORTED_INITS. */ for (init = mem_inits; init; init = TREE_CHAIN (init)) @@ -762,7 +782,7 @@ /* Issue a warning if the explicit initializer order does not match that which will actually occur. ??? Are all these on the correct lines? */ - if (warn_reorder && !subobject_init) + if (warn_reorder && !all_inits_are_const && !subobject_init) { if (TREE_CODE (TREE_PURPOSE (next_subobject)) == FIELD_DECL) warning (OPT_Wreorder, "%q+D will be initialized after",
Re: RFC: PATCH: Require and use int64 for x86 options
On 07/27/2011 06:42 PM, H.J. Lu wrote: + if (max == 64) + var_mask_1[var] = "1LL" This must be ((HOST_WIDE_INT)1). Paolo
Re: patch: don't issue -Wreorder warnings when order doesn't matter
2011/7/29 Daniel Marjamäki : > Hello! > > In my humble opinion the -Wreorder has noise. When the order doesn't > matter I would prefer that warnings are not issued. > > In this email I include a patch that I would like to get comments > about. The patch will suppress warnings if all members are initialized > with constant values. > I am not very good at GCC internals so I wonder if I made some serious > mistake when using TREE_VALUE, TREE_CODE, etc? Perhaps I overlook > something? > > Here is sample code that currently generates warnings but my patch > suppress those warnings: > > class Fred { > private: > int a; > int b; > public: > Fred() : b(0), a(0) { } > }; > > I think the next step will be to suppress the warning if both the > members that the message is about are initialized with constant > values. Why doesn't it matter in this case but it matters when the initializer are non-constant? Richard. > Best regards, > Daniel > > > Index: gcc/cp/init.c > === > --- gcc/cp/init.c (revision 176862) > +++ gcc/cp/init.c (working copy) > @@ -711,6 +711,7 @@ > VEC(tree,gc) *vbases; > int i; > int uses_unions_p; > + int all_inits_are_const; /* all members are initialized with a > constant value */ > > /* Build up a list of initializations. The TREE_PURPOSE of entry > will be the subobject (a FIELD_DECL or BINFO) to initialize. The > @@ -741,6 +742,25 @@ > without issuing a warning. */ > next_subobject = sorted_inits; > > + all_inits_are_const = 1; > + if (warn_reorder) > + { > + for (init = mem_inits; init; init = TREE_CHAIN (init)) > + { > + tree tree_value; > + > + tree_value = TREE_VALUE(init); > + if (TREE_CODE(tree_value) == TREE_LIST) > + tree_value = TREE_VALUE(tree_value); > + > + if (TREE_CODE(tree_value) != INTEGER_CST) > + { > + all_inits_are_const = 0; > + break; > + } > + } > + } > + > /* Go through the explicit initializers, filling in TREE_PURPOSE in > the SORTED_INITS. */ > for (init = mem_inits; init; init = TREE_CHAIN (init)) > @@ -762,7 +782,7 @@ > /* Issue a warning if the explicit initializer order does not > match that which will actually occur. > ??? Are all these on the correct lines? */ > - if (warn_reorder && !subobject_init) > + if (warn_reorder && !all_inits_are_const && !subobject_init) > { > if (TREE_CODE (TREE_PURPOSE (next_subobject)) == FIELD_DECL) > warning (OPT_Wreorder, "%q+D will be initialized after", >
Re: patch: don't issue -Wreorder warnings when order doesn't matter
On Fri, Jul 29, 2011 at 11:53:08AM +0200, Richard Guenther wrote: > 2011/7/29 Daniel Marjamäki : > Why doesn't it matter in this case but it matters when the initializer > are non-constant? Plus the documentation of -Wreorder even uses constants: `-Wreorder (C++ and Objective-C++ only)' Warn when the order of member initializers given in the code does not match the order in which they must be executed. For instance: struct A { int i; int j; A(): j (0), i (1) { } }; The compiler will rearrange the member initializers for `i' and `j' to match the declaration order of the members, emitting a warning to that effect. This warning is enabled by `-Wall'. If you don't want to fix up your code, just compile it with -Wno-reorder. Jakub
Defining constraint for registers tuple
Hi guys, I'm working on implementation of `mulx` (which is part of BMI2). One of improvements compared generic `mul` is that it allows to specify destination registers. For `mul` we have `A` constraint, which stands for AX:DX pair. So, is there a possibility to relax such cinstraint and allow any pair of registers as destination? Thanks, K
Re: Re: patch: don't issue -Wreorder warnings when order doesn't matter
Hello! > Why doesn't it matter in this case but it matters when the initializer > are non-constant? It doesn't matter because the program will behave the same no matter if the initializations are reordered or not. Logically it will behave just as the user expects no matter if he expects reordering or not. When one initializer is non-constant there might be a dependency between the two initializations and the wrong order might be a bug. I would like to silence such warnings also, but I don't want to try to determine if there is dependencies or not. > If you don't want to fix up your code, just compile it with -Wno-reorder. I don't want to use -Wno-reorder , because then I won't see the real problems. Don't get me wrong, I like this check. When gcc generates noise I think it is better to fix gcc than to fix my code. The only reason I can think of to keep this noise is for purely stylistic reasons. Somebody might think it is a stylistic problem to initialize members backwards. But then -Wreorder should also warn about common assignments and I doubt anybody wants that. Best regards, Daniel
Re: Re: patch: don't issue -Wreorder warnings when order doesn't matter
2011/7/29 Daniel Marjamäki : > Hello! > >> Why doesn't it matter in this case but it matters when the initializer >> are non-constant? > > It doesn't matter because the program will behave the same no matter > if the initializations are reordered or not. Logically it will behave > just as the user expects no matter if he expects reordering or not. > > When one initializer is non-constant there might be a dependency > between the two initializations and the wrong order might be a bug. > I would like to silence such warnings also, but I don't want to try to > determine if there is dependencies or not. > > >> If you don't want to fix up your code, just compile it with -Wno-reorder. > > I don't want to use -Wno-reorder , because then I won't see the real > problems. Don't get me wrong, I like this check. > > > When gcc generates noise I think it is better to fix gcc than to fix my code. > > The only reason I can think of to keep this noise is for purely > stylistic reasons. Somebody might think it is a stylistic problem to > initialize members backwards. But then -Wreorder should also warn > about common assignments and I doubt anybody wants that. Ok, I think the idea of the patch sounds reasonable then (but maybe we can do even better for more cases by adjusting the + if (TREE_CODE(tree_value) != INTEGER_CST) check to, say, if (! TREE_CONSTANT (tree_value)) which at least would also cover floating-point constants and strings. Eventually C++0x offers some more useful predicate give its constexpr feature. I suppose C++ maintainers will have to have a look here anyway. And I guess the example in the manual should be adjusted as Jakub says. Thanks, Richard. > Best regards, > Daniel >
Re: Defining constraint for registers tuple
Kirill Yukhin writes: > I'm working on implementation of `mulx` (which is part of BMI2). One > of improvements compared generic `mul` is that it allows to specify > destination registers. > For `mul` we have `A` constraint, which stands for AX:DX pair. > So, is there a possibility to relax such cinstraint and allow any pair > of registers as destination? Don't change the constraint, just add an alternative. Or use a different insn with an insn predicate. Ian
Re: Re: patch: don't issue -Wreorder warnings when order doesn't matter
2011/7/29 Richard Guenther : > 2011/7/29 Daniel Marjamäki : >> Hello! >> >>> Why doesn't it matter in this case but it matters when the initializer >>> are non-constant? >> >> It doesn't matter because the program will behave the same no matter >> if the initializations are reordered or not. Logically it will behave >> just as the user expects no matter if he expects reordering or not. >> >> When one initializer is non-constant there might be a dependency >> between the two initializations and the wrong order might be a bug. >> I would like to silence such warnings also, but I don't want to try to >> determine if there is dependencies or not. >> >> >>> If you don't want to fix up your code, just compile it with -Wno-reorder. >> >> I don't want to use -Wno-reorder , because then I won't see the real >> problems. Don't get me wrong, I like this check. >> >> >> When gcc generates noise I think it is better to fix gcc than to fix my code. >> >> The only reason I can think of to keep this noise is for purely >> stylistic reasons. Somebody might think it is a stylistic problem to >> initialize members backwards. But then -Wreorder should also warn >> about common assignments and I doubt anybody wants that. > > Ok, I think the idea of the patch sounds reasonable then (but maybe > we can do even better for more cases by adjusting the > > + if (TREE_CODE(tree_value) != INTEGER_CST) > > check to, say, if (! TREE_CONSTANT (tree_value)) which at least > would also cover floating-point constants and strings. Eventually > C++0x offers some more useful predicate give its constexpr feature. > > I suppose C++ maintainers will have to have a look here anyway. > > And I guess the example in the manual should be adjusted as Jakub > says. > > Thanks, > Richard. > >> Best regards, >> Daniel >> > Hello! Thanks for your tip. I adjusted the patch, using TREE_CONSTANT. It works. I also agree about updating the example in the manual. Best regards, Daniel Index: gcc/cp/init.c === --- gcc/cp/init.c (revision 176925) +++ gcc/cp/init.c (working copy) @@ -711,6 +711,7 @@ VEC(tree,gc) *vbases; int i; int uses_unions_p; + int all_inits_are_const; /* all members are initialized with a constant value */ /* Build up a list of initializations. The TREE_PURPOSE of entry will be the subobject (a FIELD_DECL or BINFO) to initialize. The @@ -741,6 +742,26 @@ without issuing a warning. */ next_subobject = sorted_inits; + /* check if all explicit initializations use constant values */ + all_inits_are_const = 1; + if (warn_reorder) +{ + for (init = mem_inits; init; init = TREE_CHAIN (init)) +{ + tree tree_value; + + tree_value = TREE_VALUE(init); + if (TREE_CODE(tree_value) == TREE_LIST) +tree_value = TREE_VALUE(tree_value); + + if (! TREE_CONSTANT (tree_value)) +{ + all_inits_are_const = 0; + break; +} +} +} + /* Go through the explicit initializers, filling in TREE_PURPOSE in the SORTED_INITS. */ for (init = mem_inits; init; init = TREE_CHAIN (init)) @@ -762,7 +783,7 @@ /* Issue a warning if the explicit initializer order does not match that which will actually occur. ??? Are all these on the correct lines? */ - if (warn_reorder && !subobject_init) + if (warn_reorder && !all_inits_are_const && !subobject_init) { if (TREE_CODE (TREE_PURPOSE (next_subobject)) == FIELD_DECL) warning (OPT_Wreorder, "%q+D will be initialized after",
Re: Performance degradation on g++ 4.6
My guess is inlining differences. Try more aggressive inline parameters to see if helps. Also try FDO to see there is any performance difference between two versions. You will probably need to do first level triage and file bug reports. David On Fri, Jul 29, 2011 at 10:56 AM, Oleg Smolsky wrote: > Hi there, I have compiled and run a set of C++ benchmarks on a CentOS4/64 > box using the following compilers: > a) g++4.1 that is available for this distro (GCC version 4.1.2 20071124 > (Red Hat 4.1.2-42) > b) g++4.6 that I built (stock version 4.6.1) > > The machine has two Intel quad core processors in x86_64 mode (/proc/cpuinfo > attached) > > Benchmarks were taken from this page: > http://stlab.adobe.com/performance/ > > Results: > - some of these tests showed 20..30% performance degradation > (eg the second section in the simple_types_constant_folding test: 30s > -> 44s) > - a few were quicker > - full reports are attached > > I would assume that performance of the generated code is closely monitored > by the dev community and obvious blunders should not sneak in... However, my > findings are reproducible with these synthetic benchmarks as well as > production code at work. The latter shows approximately 25% degradation on > CPU bound tests. > > Is there a trick to building the compiler or using a specific -mtune/-march > flag for my CPU? I built the compiler with all the default options (it just > has a distinct installation path): > ../gcc-%{version}/configure --prefix=/work/tools/gcc46 > --enable-languages=c,c++ --with-system-zlib --with-mpfr=/work/tools/mpfr24 > --with-gmp=/work/tools/gmp --with-mpc=/work/tools/mpc > LD_LIBRARY_PATH=/work/tools/mpfr/lib24:/work/tools/gmp/lib:/work/tools/mpc/lib > > Are there any published benchmarks? I'd appreciate any advice or pointers. > > Thanks in advance, > Oleg. >
Re: IRA vs CANNOT_CHANGE_MODE_CLASS, + 4.7 IRA regressions?
On 07/27/2011 05:59 PM, Sandra Loosemore wrote: Consider this bit of code: extern double a[20]; double test1 (int n) { double accum = 0.0; int i; for (i=0; imipsisa32r2-sde-elf-gcc -O3 -fno-inline -fno-unroll-loops -march=74kf1_1 -S abstest.c With a GCC 4.6 compiler, this produces: ... .L3: mtc1$3,$f2 ldc1$f0,0($5) addiu$5,$5,8 mtc1$2,$f3 sub.d$f2,$f2,$f0 mfc1$3,$f2 bne$5,$4,.L3 mfc1$2,$f3 ext$5,$2,0,31 move$4,$3 .L2: mtc1$4,$f0 j$31 mtc1$5,$f1 ... This is terrible code, with all that pointless register-shuffling inside the loop -- what's gone wrong? Well, the bit-twiddling expansion of "fabs" produced by optabs.c uses subreg expressions, and on MIPS CANNOT_CHANGE_MODE_CLASS disallows use of FP registers for integer operations. And, when IRA sees that, it decides it cannot alloc "accum" to a FP reg at all, even if it obviously makes sense to put it there for the rest of its lifetime. On mainline trunk, things are even worse as it's spilling to memory, not just shuffling between registers: .L3: ldc1$f0,0($2) addiu$2,$2,8 sub.d$f2,$f2,$f0 bne$2,$3,.L3 sdc1$f2,0($sp) lw$2,0($sp) ext$3,$2,0,31 lw$2,4($sp) .L2: sw$2,4($sp) sw$3,0($sp) lw$3,4($sp) lw$2,0($sp) addiu$sp,$sp,8 mtc1$3,$f0 j$31 mtc1$2,$f1 I've been experimenting with a patch to the MIPS backend to add define_insn_and_split patterns for floating-point abs -- the idea is to attach some constraints to the insns to tell IRA it needs a GP reg for these operations, so it can apply its usual cost analysis and reload logic instead of giving up. Then the split to introduce the subreg expansion happens after reload when we already have the right register class. This seems to work well enough on 4.6; for this particular example, I'm getting: .L3: ldc1$f2,0($2) addiu$2,$2,8 bne$2,$4,.L3 sub.d$f0,$f0,$f2 mfc1$2,$f1 ext$2,$2,0,31 j$31 mtc1$2,$f1 However, same patch on mainline is still giving spills to memory. :-( So, here's my question. Is it worthwhile for me to continue this approach of trying to make the MIPS backend smarter? Or is the way IRA deals with CANNOT_CHANGE_MODE_CLASS fundamentally broken and in need of fixing in a target-inspecific way? And/or is there some other regression in IRA on mainline that's causing it to spill to memory when it didn't used to in 4.6? I think the second ("fixing in a target-inspecific way"). Instead of prohibiting class for a pseudo (that what is happening for class FP_REGS) because the pseudo can change its mode, impossibility of changing mode should be reflected in the class cost (through some reload cost evaluation). I'll try to fix it. The only problem is that it will take sometime because the fix should be tested on a few platforms. It would be nice to make PR not to forget about the problem. BTW, the unary "neg" operator has the same problem as "abs" on MIPS; can't use the hardware instruction because it does the wrong thing with NaNs, and can't twiddle the sign bit directly in a FP register. With both abs/neg now generating unnecessary memory spills, this seems like a fairly important performance regression
Re: Performance degradation on g++ 4.6
Hey David, here are a couple of answers and notes: - I built the test suite with -O3 and cannot see anything else related to inlining that isn't already ON (except for -finline-limit=n which I do not how to use) http://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html - FTO looks like a very different kettle of fish, I'd prefer to leave it aside to limit the number of data points (at least for the initial investigation) - I've just rerun the suite with -flto and there are no significant differences in performance What else is there? Oleg. On 2011/7/29 11:07, Xinliang David Li wrote: My guess is inlining differences. Try more aggressive inline parameters to see if helps. Also try FDO to see there is any performance difference between two versions. You will probably need to do first level triage and file bug reports. David On Fri, Jul 29, 2011 at 10:56 AM, Oleg Smolsky wrote: Hi there, I have compiled and run a set of C++ benchmarks on a CentOS4/64 box using the following compilers: a) g++4.1 that is available for this distro (GCC version 4.1.2 20071124 (Red Hat 4.1.2-42) b) g++4.6 that I built (stock version 4.6.1) The machine has two Intel quad core processors in x86_64 mode (/proc/cpuinfo attached) Benchmarks were taken from this page: http://stlab.adobe.com/performance/ Results: - some of these tests showed 20..30% performance degradation (eg the second section in the simple_types_constant_folding test: 30s -> 44s) - a few were quicker - full reports are attached I would assume that performance of the generated code is closely monitored by the dev community and obvious blunders should not sneak in... However, my findings are reproducible with these synthetic benchmarks as well as production code at work. The latter shows approximately 25% degradation on CPU bound tests. Is there a trick to building the compiler or using a specific -mtune/-march flag for my CPU? I built the compiler with all the default options (it just has a distinct installation path): ../gcc-%{version}/configure --prefix=/work/tools/gcc46 --enable-languages=c,c++ --with-system-zlib --with-mpfr=/work/tools/mpfr24 --with-gmp=/work/tools/gmp --with-mpc=/work/tools/mpc LD_LIBRARY_PATH=/work/tools/mpfr/lib24:/work/tools/gmp/lib:/work/tools/mpc/lib Are there any published benchmarks? I'd appreciate any advice or pointers. Thanks in advance, Oleg.
Re: Performance degradation on g++ 4.6
On Fri, Jul 29, 2011 at 11:57 AM, Oleg Smolsky wrote: > Hey David, here are a couple of answers and notes: > - I built the test suite with -O3 and cannot see anything else related to > inlining that isn't already ON (except for -finline-limit=n which I do not > how to use) size estimation, inline heuristics are different between two versions, so it won't be surprising they make different decisions. Profiling tools are your best friend here. If you don't have access to any, the least you can do is to build the program with -pg option and use gprof tool to find out differences. David > http://gcc.gnu.org/onlinedocs/gcc/Optimize-Options.html > - FTO looks like a very different kettle of fish, I'd prefer to leave it > aside to limit the number of data points (at least for the initial > investigation) > - I've just rerun the suite with -flto and there are no significant > differences in performance > > What else is there? > > Oleg. > > On 2011/7/29 11:07, Xinliang David Li wrote: >> >> My guess is inlining differences. Try more aggressive inline >> parameters to see if helps. Also try FDO to see there is any >> performance difference between two versions. You will probably need to >> do first level triage and file bug reports. >> >> David >> >> >> On Fri, Jul 29, 2011 at 10:56 AM, Oleg Smolsky >> wrote: >>> >>> Hi there, I have compiled and run a set of C++ benchmarks on a CentOS4/64 >>> box using the following compilers: >>> a) g++4.1 that is available for this distro (GCC version 4.1.2 >>> 20071124 >>> (Red Hat 4.1.2-42) >>> b) g++4.6 that I built (stock version 4.6.1) >>> >>> The machine has two Intel quad core processors in x86_64 mode >>> (/proc/cpuinfo >>> attached) >>> >>> Benchmarks were taken from this page: >>> http://stlab.adobe.com/performance/ >>> >>> Results: >>> - some of these tests showed 20..30% performance degradation >>> (eg the second section in the simple_types_constant_folding test: >>> 30s >>> -> 44s) >>> - a few were quicker >>> - full reports are attached >>> >>> I would assume that performance of the generated code is closely >>> monitored >>> by the dev community and obvious blunders should not sneak in... However, >>> my >>> findings are reproducible with these synthetic benchmarks as well as >>> production code at work. The latter shows approximately 25% degradation >>> on >>> CPU bound tests. >>> >>> Is there a trick to building the compiler or using a specific >>> -mtune/-march >>> flag for my CPU? I built the compiler with all the default options (it >>> just >>> has a distinct installation path): >>> ../gcc-%{version}/configure --prefix=/work/tools/gcc46 >>> --enable-languages=c,c++ --with-system-zlib >>> --with-mpfr=/work/tools/mpfr24 >>> --with-gmp=/work/tools/gmp --with-mpc=/work/tools/mpc >>> >>> LD_LIBRARY_PATH=/work/tools/mpfr/lib24:/work/tools/gmp/lib:/work/tools/mpc/lib >>> >>> Are there any published benchmarks? I'd appreciate any advice or >>> pointers. >>> >>> Thanks in advance, >>> Oleg. >>> > >
Re: X32 project status update
Hi, This is the x32 project status update: https://sites.google.com/site/x32abi/ We have settled down on a preliminary system call number scheme: 1. In kernel, x86-64 and x32 share the same system call slot if they are the same. 2. First 512 system call slots are reserved for common and x86-64 specific ones. X32 specific system call slots start at 512. 3. All x32 system call numbers have the bit 30 set. Gibc x32 binary rpms for Fedora 15/x86-64 are available from the x32 website. People can try out x32 on Fedora 15. Thanks. -- H.J.
gcc-4.6-20110729 is now available
Snapshot gcc-4.6-20110729 is now available on ftp://gcc.gnu.org/pub/gcc/snapshots/4.6-20110729/ and on various mirrors, see http://gcc.gnu.org/mirrors.html for details. This snapshot has been generated from the GCC 4.6 SVN branch with the following options: svn://gcc.gnu.org/svn/gcc/branches/gcc-4_6-branch revision 176952 You'll find: gcc-4.6-20110729.tar.bz2 Complete GCC MD5=5709b461375ecf3eb0d2b104e5f700a1 SHA1=47e69ea16fadf9b7ab6e29d83319def1ad328865 Diffs from 4.6-20110722 are available in the diffs/ subdirectory. When a particular snapshot is ready for public consumption the LATEST-4.6 link is updated and a message is sent to the gcc list. Please do not use a snapshot before it has been announced that way.