Re: Should -Wmaybe-uninitialized be included in -Wall?
On Thu, Jul 11, 2013 at 12:42 AM, Ryan Hill wrote: > On Wed, 10 Jul 2013 07:49:18 -0500 > Gabriel Dos Reis wrote: > >> If we include a warning in -Wall then it is because we believe it to be >> generally useful and likely to uncover common bugs/mistakes. It is therefore >> reasonable for users to issue -Wall -Werror even in application delivery >> mode. > > Arg, no. -Werror is very useful for development and I'm sure that code > quality increases because of it, but it should never be enabled by default for > releases. I think about 80% of the bugs we've had filed so far for packages > failing to build against 4.8 are due to -Werror. The fact that you have 80% failing to build is not in itself an argument for not including -Werror in release mode. The real issue is whether those warnings uncovered any real bugs. If they don't, then either we are emitting too many false positives, or those packages should have turned off the offending diagnostics. > Also, several distros patch > gcc to enable additional warnings by default (eg. Debian, Ubuntu, and Gentoo > enable -Wformat=security) that upstream may not see or be interested in. It's > a > big enough headache that we had to ban use of -Werror from our tree (instead > we > flag important warnings and output them at the end of the build). Well, the issue isn't as clear cut as you make it sound. Some distros build service monitor compiler outputs for some of these warnings and abort builds even if the package does not supply -Werror. -- Gaby
Re: Should -Wmaybe-uninitialized be included in -Wall?
Jeff Law writes: > On 07/10/2013 04:51 AM, Andreas Arnez wrote: >> OK, I may be biased, because I have *only* seen false positives with >> this warning so far. Others may have made better experience with it. > It's found numerous bugs across many projects. The reduction in bug > reports against GCC which turn out to be uninitialized variables in > the user's code has dropped dramatically over the last decade (if only > the same could be said for user malloc errors reported against glibc > :( Interesting. Upstream GCC seems to have added this option to -Wall on April 26th in 2011. Have those projects been using the option explicitly before? > What you're seeing may be an indication that most of the real bugs > have been found & squashed and what remains may be mostly false > positives. Actually, I believe most of the "maybe uninitialized" false positives I saw were with code that hasn't been touched since many years. They suddenly appeared due to the introduction of this option in "-Wall" or due to regressions in GCC. >> My understanding of -Wall (so far) was that it shouldn't include >> warnings with false positives. > No, that's not true at all. -Wall is a set of warnings that the GCC > developers have found useful over time and strive to avoid triggering > in our own code. We've also seen that -Wall maps reasonably well to > problems other projects want to fix in their own code. > > We certainly evaluate whether or not the warning generates too many > false positives when we decide whether or not to include it in > -Wall. However, we certainly allow warnings which generate false > positives to be in -Wall. Then maybe that's what should be documented for "-Wall" instead of the current (misleading) description. Particuarly I suggest to add a disclaimer about possible false positives, and to remove the "easy to avoid". So instead of this: This enables all the warnings about constructions that some users consider questionable, and that are easy to avoid (or modify to prevent the warning), even in conjunction with macros. Maybe more something like this: This enables all the warnings about constructions that GCC developers consider questionable, or which GCC isn't sure about whether they might be questionable.
RE: Inter register constraints
> -Original Message- > From: Georg-Johann Lay [mailto:a...@gjlay.de] > Sent: 05 July 2013 18:03 > To: Paulo Matos > Cc: gcc@gcc.gnu.org > Subject: Re: Inter register constraints > > > have 64 registers that will give you 22 pairs. I could, of course, > > create all of these by hand by defining 23 classes and define a > > single constraint that matches these classes but I would like to know > > if there's another way. > > What are you trying to achieve? In order so synthesize MOVW > instructions after reload, see respective RTL peepholes. > Thanks Johann, that seems to be pretty much what I want to do. However when, in avr, there's a HI move of a register pair only the first register shows up in the instruction. So movw r0:r1, r2:r3 is a *movhi: (set (reg:HI r0) (reg:HI r2)) How does gcc know that r1 is going to be clobbered? Is it because GET_MODE_SIZE (HImode) is twice the register size and so it assumed the following register is clobbered as well? (or is there any hook that needs to be set) Paulo Matos
Re: Should -Wmaybe-uninitialized be included in -Wall?
On Thu, Jul 11, 2013 at 11:11:28AM +0200, Andreas Arnez wrote: > > On 07/10/2013 04:51 AM, Andreas Arnez wrote: > >> OK, I may be biased, because I have *only* seen false positives with > >> this warning so far. Others may have made better experience with it. > > It's found numerous bugs across many projects. The reduction in bug > > reports against GCC which turn out to be uninitialized variables in > > the user's code has dropped dramatically over the last decade (if only > > the same could be said for user malloc errors reported against glibc > > :( > > Interesting. Upstream GCC seems to have added this option to -Wall on > April 26th in 2011. Have those projects been using the option > explicitly before? You are misreading that change. On April 26th 2011 the -Wuninitialized option was just split into -Wuninitialized and -Wmaybe-uninitialized, to allow people to specify e.g. -Wno-error=maybe-uninitialized. So the warning was included in -Wall before as well, for many many years. Jakub
Re: Calculating instruction costs
Hi, On Wed, 10 Jul 2013, David Given wrote: > Michael Matz wrote: > [...] > > As you didn't adjust any cost I would guess the high value comes from the > > default implementation of address_cost, which simply uses arithmetic cost, > > and the MULT in there is quite expensive by default. > > > > See TARGET_ADDRESS_COST in several ports. > > Oddly, TARGET_ADDRESS_COST is never being called for my port, Bah, it's only used from some call sites (fwprop, postreload and loop-invariant), not generally to adjust the cost of (MEM addr) based on address_cost (addr). That might be a sensible change in itself I guess. > After having done a bunch of reading up on how costing works, and > deciphering the rather cryptic other ports, my understanding is: > > Costing is based entirely on analysis of the RTL, and is completely > irrelevant of what insns are selected. > > Therefore if my backend wants to support certain optimised addressing > modes, I need to insert code into my TARGET_RTX_COSTS hook that looks > for mem constructions which can be represented by such addressing modes, (or for when outer_code is MEM) > and encourages the compiler to select them by giving them a low cost. I > don't get any assistance from the patterns in the .md file. > > Have I got that right? With the current state of affairs, yes. Ciao, Michael.
Re: Should -Wmaybe-uninitialized be included in -Wall?
Hi, On Thu, 11 Jul 2013, Gabriel Dos Reis wrote: > > Arg, no. -Werror is very useful for development and I'm sure that > > code quality increases because of it, but it should never be enabled > > by default for releases. I think about 80% of the bugs we've had > > filed so far for packages failing to build against 4.8 are due to > > -Werror. > > The fact that you have 80% failing to build is not in itself an argument > for not including -Werror in release mode. The real issue is whether > those warnings uncovered any real bugs. If they don't, then either we > are emitting too many false positives, or those packages should have > turned off the offending diagnostics. Not quite. The problem is that the compiler is used for two completely different things: 1) by developers to develop their code, a development too. They will use a certain version of GCC, hence enabling -Wall -Werror is indeed good practice; even with the expectation that the developer will eventually change the used GCC version implying different errors; this will merely result in the source base to be improved by that developer to also conform to the new warnings (and hopefully not in a way that it warns with the old compiler again) 2) by users of software (which includes distributors as one of the largest consumers of random source codes) as sort of install tool. This will most of the time _not_ be the version of the compiler that the developer used when the software was released. As we already determined adding -Werror effectively makes the software require a very specific GCC version, but only because of warnings. Individual consumers of software usually are not inherently interested in such warnings, and distributors have different ways of enforcing policies related to source code quality (e.g. by parsing build logs). Adding -Werror by developers for their released software is hence neither required, nor even advised, it'll merely make life harder for consumers. Then, of course, there's a mixture between consumers and developers, who might indeed be interested in warnings with the intention of helping the sources developers. Well, as they are developers themself, they should know a way to add -Werror themself. So, no, -Werror for delivered stuff is no good idea. > > Also, several distros patch gcc to enable additional warnings by > > default (eg. Debian, Ubuntu, and Gentoo enable -Wformat=security) that > > upstream may not see or be interested in. It's a big enough headache > > that we had to ban use of -Werror from our tree (instead we flag > > important warnings and output them at the end of the build). > > Well, the issue isn't as clear cut as you make it sound. Some distros > build service > monitor compiler outputs for some of these warnings and abort builds > even if the package does not supply -Werror. Yes, one reason why -Werror for application delivery is unnecessary. Ciao, Michael.
rl78-elf: Compilation broken due to missing constraint
Hi! I guess you forgot a small patch to constraints.md, because since this commit which uses a new "U" constraint, the rl78-elf target won't build: 2013-05-31 Kaushik Phatak * config/rl78/rl78.md (mulqi3,mulhi3): New define_expands. (*mulqi3_rl78,*mulhi3_rl78,*mulhi3_g13): New define_insns. [...] g++ -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -fno-exceptions -fno-rtti -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings -fno-common -DHAVE_CONFIG_H -DGENERATOR_FILE -static-libstdc++ -static-libgcc -o build/genoutput \ build/genoutput.o build/rtl.o build/read-rtl.o build/ggc-none.o build/vec.o build/min-insn-modes.o build/gensupport.o build/print-rtl.o build/read-md.o build/errors.o ../build-x86_64-unknown-linux-gnu/libiberty/libiberty.a build/genoutput ../../../../gcc/gcc/config/rl78/rl78.md \ insn-conditions.md > tmp-output.c ../../../../gcc/gcc/config/rl78/rl78.md:265: error: undefined machine-specific constraint at this point: "U" ../../../../gcc/gcc/config/rl78/rl78.md:265: note: in operand 1 ../../../../gcc/gcc/config/rl78/rl78.md:282: error: undefined machine-specific constraint at this point: "U" ../../../../gcc/gcc/config/rl78/rl78.md:282: note: in operand 1 ../../../../gcc/gcc/config/rl78/rl78.md:297: error: undefined machine-specific constraint at this point: "U" ../../../../gcc/gcc/config/rl78/rl78.md:297: note: in operand 1 make[2]: *** [s-output] Error 1 MfG, JBG -- Jan-Benedict Glaw jbg...@lug-owl.de +49-172-7608481 Signature of:Lauf nicht vor Deinem Glück davon: the second : Es könnte hinter Dir stehen! signature.asc Description: Digital signature
Re: Porting from old to new GCC versions
On 7/11/13 4:23 AM, Hendrik Greving wrote: Hi, I have a hard time finding a good description of how old, obsolete and now poisoned target macros and backend switches had been replaced with. Examples are TARGET_SWITCHES, or CAN_DEBUG_WITHOUT_FP. I am porting from a very old compiler version. Is there any documentation (except ChangeLog which doesn't say much) available for these kind of patches? Regards, Hendrik Greving IMHO, the GCC Internals is the best documentation for your reference. Some obsolete macros are explicitly described in chapter 17.9. Other deprecated macros spread out over chapter 17 (e.g. FUNCTION_VALUE). My suggestion is to review your target macros one by one. See if they still can be found in GCC Internals. Best regards, jasonwucj
Re: HAVE_ATTR_enabled mishandling?
On 7/10/13 5:51 AM, David Given wrote: I think I have found a bug. This is in stock gcc 4.8.1... My backend does not use the 'enabled' attribute; therefore the following code in insn-attr.h kicks in: #ifndef HAVE_ATTR_enabled #define HAVE_ATTR_enabled 0 #endif Therefore the following code in gcc/lra-constraints.c is enabled: #ifdef HAVE_ATTR_enabled if (curr_id->alternative_enabled_p != NULL && ! curr_id->alternative_enabled_p[nalt]) continue; #endif ->alternative_enabled_p is bogus; therefore segfault. Elsewhere I see structures of the form: #if HAVE_ATTR_enabled ... #endif So I think that #ifdef above is a straight typo. Certainly, changing it to a #if makes the crash go away... I faced exactly the same problem as yours. When I was trying to enable LRA on my target, I got segfault as well because I did not have 'enabled' attribute in my target.md file. I was told it was due to my faulty md design. The simplest workaround is to define 'enabled' attribute so I took it. But I suspected that may be a latent issue. It's so happy to see others have the same question and provide a solution. Certainly, whether your solution is correct should be reviewed by LRA maintainer, Vladmir Makarov, so I cc him in the list. Best regards, jasonwucj
gcc-4.8-20130711 is now available
Snapshot gcc-4.8-20130711 is now available on ftp://gcc.gnu.org/pub/gcc/snapshots/4.8-20130711/ and on various mirrors, see http://gcc.gnu.org/mirrors.html for details. This snapshot has been generated from the GCC 4.8 SVN branch with the following options: svn://gcc.gnu.org/svn/gcc/branches/gcc-4_8-branch revision 200914 You'll find: gcc-4.8-20130711.tar.bz2 Complete GCC MD5=2cc6935598ae77af7b99b028091cc1c7 SHA1=574895a5abc2f2edac94f9fad719fdace9cfaf04 Diffs from 4.8-20130704 are available in the diffs/ subdirectory. When a particular snapshot is ready for public consumption the LATEST-4.8 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.
RE: rl78-elf: Compilation broken due to missing constraint
> I guess you forgot a small patch to constraints.md, because since this commit > which uses a new "U" constraint, the rl78-elf target won't > build: > 2013-05-31 Kaushik Phatak > > * config/rl78/rl78.md (mulqi3,mulhi3): New define_expands. > (*mulqi3_rl78,*mulhi3_rl78,*mulhi3_g13): New define_insns. Yes, DJ did point out this missing constraint in my patch. I have posted this and committed this patch last month itself, http://gcc.gnu.org/ml/gcc-patches/2013-06/msg00643.html It does seem to have got submitted to gcc-cvs and I am able to see this change when I do a svn update, http://gcc.gnu.org/ml/gcc-cvs/2013-06/msg00409.html Please correct me if I am wrong on this. Best Regards, Kaushik
