Message ID | oroc2k34i5.fsf@livre.localdomain |
---|---|
State | New |
Headers | show |
On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote: > In an -O3 bootstrap, out.clauses are reported as uninitialized. GCC is > not smart enough to realize accesses to the uninitialized members will > never happen. It shouldn't be too expensive to initialize them all, so > this is what this patch does. Regstrapped on x86_64-linux-gnu and > i686-linux-gnu. Ok to install? Oops, I mistakenly checked this in along with a couple of other approved patches, sorry. Should I back it out, or may I leave it in? (please :-) > for gcc/ChangeLog > from Alexandre Oliva <aoliva@redhat.com> > * ipa-inline-analysis.c (read_predicate): Initialize all clauses. > Index: gcc/ipa-inline-analysis.c > =================================================================== > --- gcc/ipa-inline-analysis.c.orig 2011-05-18 03:29:08.295328903 -0300 > +++ gcc/ipa-inline-analysis.c 2011-05-18 03:29:38.187242177 -0300 > @@ -2289,6 +2289,11 @@ read_predicate (struct lto_input_block * > clause = out.clause[k++] = lto_input_uleb128 (ib); > } > while (clause); > + > + /* Zero-initialize the remaining clauses in OUT. */ > + while (k <= MAX_CLAUSES) > + out.clause[k++] = 0; > + > return out; > }
> On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote: > > > In an -O3 bootstrap, out.clauses are reported as uninitialized. GCC is > > not smart enough to realize accesses to the uninitialized members will > > never happen. It shouldn't be too expensive to initialize them all, so > > this is what this patch does. Regstrapped on x86_64-linux-gnu and > > i686-linux-gnu. Ok to install? > > Oops, I mistakenly checked this in along with a couple of other approved > patches, sorry. > > Should I back it out, or may I leave it in? (please :-) > > > for gcc/ChangeLog > > from Alexandre Oliva <aoliva@redhat.com> > > > * ipa-inline-analysis.c (read_predicate): Initialize all clauses. > > > Index: gcc/ipa-inline-analysis.c > > =================================================================== > > --- gcc/ipa-inline-analysis.c.orig 2011-05-18 03:29:08.295328903 -0300 > > +++ gcc/ipa-inline-analysis.c 2011-05-18 03:29:38.187242177 -0300 > > @@ -2289,6 +2289,11 @@ read_predicate (struct lto_input_block * > > clause = out.clause[k++] = lto_input_uleb128 (ib); > > } > > while (clause); > > + > > + /* Zero-initialize the remaining clauses in OUT. */ > > + while (k <= MAX_CLAUSES) > > + out.clause[k++] = 0; Well, clauses is a typical zero terminated array, so I really do think the warning is bogus. Do you at least know why we end up with that weird warning? Honza
> On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote: > > > In an -O3 bootstrap, out.clauses are reported as uninitialized. GCC is > > not smart enough to realize accesses to the uninitialized members will > > never happen. It shouldn't be too expensive to initialize them all, so > > this is what this patch does. Regstrapped on x86_64-linux-gnu and > > i686-linux-gnu. Ok to install? > > Oops, I mistakenly checked this in along with a couple of other approved > patches, sorry. > > Should I back it out, or may I leave it in? (please :-) But concerning backing out, I don't see why we should break bootstrap again. it would be however nice to get a testcase and track the bogus warning issue. Thanks! Honza
On Jun 6, 2011, Jan Hubicka <hubicka@ucw.cz> wrote: >> On May 30, 2011, Alexandre Oliva <aoliva@redhat.com> wrote: >> > Index: gcc/ipa-inline-analysis.c > Well, clauses is a typical zero terminated array, so I really do think > the warning is bogus. Looking into it, I'm not sure it is. Compiling with -O3 I get: ipa-inline-analysis.c: In function ‘inline_read_summary’: ipa-inline-analysis.c:531:22: warning: ‘out.clause[8]’ may be used uninitialized in this function [-Wmaybe-uninitialized] ipa-inline-analysis.c:2294:20: note: ‘out.clause[8]’ was declared here ... > Do you at least know why we end up with that weird warning? 531: *es->predicate = *predicate; This is clearly copying all fields and array members, even the uninitialized ones. However, like padding copied while copying structs, I'm not sure that's a problem if the copied members are not accessed. However, it is absolutely correct that we are copying uninitialized data members.
for gcc/ChangeLog from Alexandre Oliva <aoliva@redhat.com> * ipa-inline-analysis.c (read_predicate): Initialize all clauses. Index: gcc/ipa-inline-analysis.c =================================================================== --- gcc/ipa-inline-analysis.c.orig 2011-05-18 03:29:08.295328903 -0300 +++ gcc/ipa-inline-analysis.c 2011-05-18 03:29:38.187242177 -0300 @@ -2289,6 +2289,11 @@ read_predicate (struct lto_input_block * clause = out.clause[k++] = lto_input_uleb128 (ib); } while (clause); + + /* Zero-initialize the remaining clauses in OUT. */ + while (k <= MAX_CLAUSES) + out.clause[k++] = 0; + return out; }