Bug 36168 - bogus uninitialized warning (huge testcase)
Summary: bogus uninitialized warning (huge testcase)
Status: RESOLVED WORKSFORME
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.4.0
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks: Wuninitialized
  Show dependency treegraph
 
Reported: 2008-05-07 09:33 UTC by Martin Reinecke
Modified: 2009-02-09 16:15 UTC (History)
2 users (show)

See Also:
Host: i686-pc-linux-gnu
Target: i686-pc-linux-gnu
Build: i686-pc-linux-gnu
Known to work:
Known to fail:
Last reconfirmed: 2008-05-07 10:12:31


Attachments
a (not really reduced) test case (118.42 KB, application/gzip)
2008-05-07 09:35 UTC, Martin Reinecke
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Reinecke 2008-05-07 09:33:42 UTC
When compiling the attached testcase with current mainline, bogus warnings are emitted:

/scratch/martin/splotch>g++ -v -O -Wuninitialized bug.ii
Using built-in specs.
Target: i686-pc-linux-gnu
Configured with: /scratch/martin/gcc/configure --prefix=/afs/mpa/data/martin/ugcc --with-mpfr-include=/usr/include --with-mpfr-lib=/usr/lib --with-gmp-include=/usr/include --with-gmp-lib=/usr/lib --enable-languages=c++,fortran --enable-checking=release
Thread model: posix
gcc version 4.4.0 20080507 (experimental) [trunk revision 135032] (GCC) 
COLLECT_GCC_OPTIONS='-v' '-O' '-Wuninitialized' '-shared-libgcc' '-mtune=generic'
 /afs/mpa/data/martin/ugcc/libexec/gcc/i686-pc-linux-gnu/4.4.0/cc1plus -fpreprocessed bug.ii -quiet -dumpbase bug.ii -mtune=generic -auxbase bug -O -Wuninitialized -version -o /tmp/ccSh8Ooh.s
GNU C++ (GCC) version 4.4.0 20080507 (experimental) [trunk revision 135032] (i686-pc-linux-gnu)
        compiled by GNU C version 4.4.0 20080507 (experimental) [trunk revision 135032], GMP version 4.2.1, MPFR version 2.3.1.
warning: GMP header version 4.2.1 differs from library version 4.2.2.
GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: d49e4fa2e0d6ffd2b8e9abb2dcc5530c
In file included from splotch/splotch.cc:37,
                 from fullsplotch.cc:5:
./splotch/splotchutils.h: In member function 'void splotch_renderer::render(const std::vector<particle2, std::allocator<particle2> >&, arr2<RAYPP::COLOUR>&, bool, double)':
./splotch/splotchutils.h:132: warning: 'q.COLOUR8::r' may be used uninitialized in this function
./splotch/splotchutils.h:132: warning: 'q.COLOUR8::g' may be used uninitialized in this function
COLLECT_GCC_OPTIONS='-v' '-O' '-Wuninitialized' '-shared-libgcc' '-mtune=generic'
 as -V -Qy -o /tmp/ccPwUfT2.o /tmp/ccSh8Ooh.s
GNU assembler version 2.18 (i686-pc-linux-gnu) using BFD version (GNU Binutils) 2.18
COMPILER_PATH=/afs/mpa/data/martin/ugcc/libexec/gcc/i686-pc-linux-gnu/4.4.0/:/afs/mpa/data/martin/ugcc/libexec/gcc/i686-pc-linux-gnu/4.4.0/:/afs/mpa/data/martin/ugcc/libexec/gcc/i686-pc-linux-gnu/:/afs/mpa/data/martin/ugcc/lib/gcc/i686-pc-linux-gnu/4.4.0/:/afs/mpa/data/martin/ugcc/lib/gcc/i686-pc-linux-gnu/:/usr/libexec/gcc/i686-pc-linux-gnu/:/usr/lib/gcc/i686-pc-linux-gnu/
LIBRARY_PATH=/afs/mpa/data/martin/ugcc/lib/gcc/i686-pc-linux-gnu/4.4.0/:/afs/mpa/data/martin/ugcc/lib/gcc/i686-pc-linux-gnu/4.4.0/../../../:/lib/:/usr/lib/
COLLECT_GCC_OPTIONS='-v' '-O' '-Wuninitialized' '-shared-libgcc' '-mtune=generic'
 /afs/mpa/data/martin/ugcc/libexec/gcc/i686-pc-linux-gnu/4.4.0/collect2 --eh-frame-hdr -m elf_i386 -dynamic-linker /lib/ld-linux.so.2 /usr/lib/crt1.o /usr/lib/crti.o /afs/mpa/data/martin/ugcc/lib/gcc/i686-pc-linux-gnu/4.4.0/crtbegin.o -L/afs/mpa/data/martin/ugcc/lib/gcc/i686-pc-linux-gnu/4.4.0 -L/afs/mpa/data/martin/ugcc/lib/gcc/i686-pc-linux-gnu/4.4.0/../../.. /tmp/ccPwUfT2.o -lstdc++ -lm -lgcc_s -lgcc -lc -lgcc_s -lgcc /afs/mpa/data/martin/ugcc/lib/gcc/i686-pc-linux-gnu/4.4.0/crtend.o /usr/lib/crtn.o

There are several funny things happening here:
 - in principle the compiler is able to detect that "q" will not be used uninitialized; if I remove pieces of code somewhere completely different in the testcase, the warning dieappears
 - the syntax "'q.COLOUR8::r' may be used uninitialized in this function" is unexpected; typically the warnings look different
 - whether the warning is printed or not depends very sensitively on the surrounding code, so it's very hard to produce a small testcase. I have the feeling that it is only emitted if the processed function exceeds a certain complexity; but this is just a guess.

I have observed this behaviour in older versions, back to (at least) 4.2, but did not open a PR about it before, since I wasn't able to provide a good testcase. I hope the attached one is not completely useless ...
Comment 1 Martin Reinecke 2008-05-07 09:35:00 UTC
Created attachment 15590 [details]
a (not really reduced) test case
Comment 2 Andrew Pinski 2008-05-07 09:43:18 UTC
This is a normal issue with the unitialized warnings.  See PR 5035.  Basically to get this warning correct for this case, you need conditional PHIs which we don't have currently.  And I don't know of any compiler that does.  

Basically the code looks like:

if (a)
 set b
for(...)
  for(...)
    if (a)
      use b

Unswitching the loops will help somewhat as then you can then jump thread.

Thanks,
Andrew Pinski
Comment 3 Andrew Pinski 2008-05-07 09:51:07 UTC
Also you may as well manually unswitch the loops as they don't do anything except some multiplication if that bool is true.  That is better to write the code as:
           if (a_eq_e)
            return;
          e=p[m].e;
          q=COLOUR8(e.r/(a.r+grayabsorb),e.g/(a.g+grayabsorb),e.b/(a.b+grayabsorb));
          float64 radsq = rfacr*rfacr;
          float64 prefac1 = -0.5/(r*r*sigma0*sigma0);
          float64 prefac2 = -0.5*bfak/p[m].ro;
            {
            for (int x=minx; x<maxx; ++x)
              {
              float64 xsq=(x-posx)*(x-posx);
              for (int y=miny; y<maxy; ++y)
                {
                float64 dsq = (y-posy)*(y-posy) + xsq;
                if (dsq<radsq)
                  {
                  lpic[x][y].r = q.r;
                  lpic[x][y].g = q.g;
                  }
                }
              }
            }

As you now have better code as the code is faster as you don't have to go through those loops at all or even calculate the inner float64s.
Comment 4 Martin Reinecke 2008-05-07 09:51:45 UTC
It would be completely fine by me, if g++ simply emitted bogus warnings in a consistent way. But the syntax is still confusing, and what seems quite disconcerting to me is the fact that _both_ warnings disappear if I comment the line 37214 (lpic[x][y].g = q.g;). Is this also expected behaviour?
Comment 5 Andrew Pinski 2008-05-07 09:54:10 UTC
(In reply to comment #4)
> Is this also expected behavior?

Most likely because SRA choses not to scalarize the aggregate.  Aka the optimizators are choosing different choses based on the code.   Nothing new.

-- Pinski
Comment 6 Martin Reinecke 2008-05-07 09:57:09 UTC
OK. Thanks for the clarification!
Comment 7 Manuel López-Ibáñez 2008-05-07 10:12:31 UTC
This would be more consistent if uninitialized warnings would work in VOPs.

Anyway, I think we should keep this open as an interesting testcase.
Comment 8 Manuel López-Ibáñez 2008-08-29 03:51:40 UTC
Things TODO here:

* Find out if we can get the "original form" of 'q.COLOUR8::r' to print something closer to the original code.

* What is the difference in SSA after SRA when line 37214 is commented out?

* Perhaps we should never warn about things like this. Can we mark them as artificial? If not, try setting and preserving TREE_NO_WARNING.

* Does this have to do with compiler-generated EH? If so, this is a duplicate of another PR I have seen.

* The attached testcase is too big for the testsuite. We would need a smaller one. Unfortunately, automatically reducing the testcase would probably lead to a bogus one. So it must be done manually or find an alternative testcase.

Comment 9 Andrew Pinski 2008-12-28 05:12:16 UTC
>* Find out if we can get the "original form" of 'q.COLOUR8::r' to print
something closer to the original code.

Actually this is a good thing we print out "COLOUR8::" really, sometimes there are different r's in a class so knowing which one is being used here makes it easy to understand.
Comment 10 Manuel López-Ibáñez 2009-02-09 16:13:01 UTC
I cannot reproduce this with current GCC 4.4

Also, the testcase is too big.
Comment 11 Manuel López-Ibáñez 2009-02-09 16:15:34 UTC
Actually, I am going to close it as WORKSFORME, but if you can reproduce this with a GCC later than revision 143971 (even in this huge testcase), please reopen. Thanks for the report.