diff mbox

[bootstrap-O3] add a default initializer to avoid a warning at -O3

Message ID orshp03e5i.fsf@lxoliva.fsfla.org
State New
Headers show

Commit Message

Alexandre Oliva Jan. 3, 2017, 5:28 a.m. UTC
Building with the bootstrap-O3 configuration option fails to compile
input.c due to an AFAICT false-positive warning about an uninitialized
use of a variable.

This patch adds a default initializer to silence it.

Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  OK to install?

for  gcc/ChangeLog

	* input.c (assert_char_at_range): Default-initialize
	actual_range.
---
 gcc/input.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Jeff Law Jan. 3, 2017, 5:45 p.m. UTC | #1
On 01/02/2017 10:28 PM, Alexandre Oliva wrote:
> Building with the bootstrap-O3 configuration option fails to compile
> input.c due to an AFAICT false-positive warning about an uninitialized
> use of a variable.
>
> This patch adds a default initializer to silence it.
>
> Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  OK to install?
>
> for  gcc/ChangeLog
>
> 	* input.c (assert_char_at_range): Default-initialize
> 	actual_range.
OK.
jeff
Jason Merrill Jan. 3, 2017, 7:23 p.m. UTC | #2
Are there bugzillas for these false positive warnings?

On Tue, Jan 3, 2017 at 12:45 PM, Jeff Law <law@redhat.com> wrote:
> On 01/02/2017 10:28 PM, Alexandre Oliva wrote:
>>
>> Building with the bootstrap-O3 configuration option fails to compile
>> input.c due to an AFAICT false-positive warning about an uninitialized
>> use of a variable.
>>
>> This patch adds a default initializer to silence it.
>>
>> Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  OK to install?
>>
>> for  gcc/ChangeLog
>>
>>         * input.c (assert_char_at_range): Default-initialize
>>         actual_range.
>
> OK.
> jeff
>
Alexandre Oliva Jan. 4, 2017, 1:13 p.m. UTC | #3
On Jan  3, 2017, Jason Merrill <jason@redhat.com> wrote:

> Are there bugzillas for these false positive warnings?

I don't think so.

Did you mean in the sense that the patch silences them and thus "fixes"
them, or in the sense that the underlying cause for the warnings is not
fixed and someone might want to look into them at a later time?

FWIW, I hadn't considered the latter (it doesn't seem too hard to find
such warnings anyway; every time I try bootstrap-O3, which is not very
often, I get a few of those), and the former didn't seem enough of a
reason to have bug reports created.
Jason Merrill Jan. 4, 2017, 1:42 p.m. UTC | #4
On Wed, Jan 4, 2017 at 8:13 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
> On Jan  3, 2017, Jason Merrill <jason@redhat.com> wrote:
>
>> Are there bugzillas for these false positive warnings?
>
> I don't think so.
>
> Did you mean in the sense that the patch silences them and thus "fixes"
> them, or in the sense that the underlying cause for the warnings is not
> fixed and someone might want to look into them at a later time?
>
> FWIW, I hadn't considered the latter (it doesn't seem too hard to find
> such warnings anyway; every time I try bootstrap-O3, which is not very
> often, I get a few of those), and the former didn't seem enough of a
> reason to have bug reports created.

The latter; false positive warnings like this are bugs that should be fixed.

Jason
Jeff Law Jan. 4, 2017, 4:04 p.m. UTC | #5
On 01/04/2017 06:42 AM, Jason Merrill wrote:
> On Wed, Jan 4, 2017 at 8:13 AM, Alexandre Oliva <aoliva@redhat.com> wrote:
>> On Jan  3, 2017, Jason Merrill <jason@redhat.com> wrote:
>>
>>> Are there bugzillas for these false positive warnings?
>>
>> I don't think so.
>>
>> Did you mean in the sense that the patch silences them and thus "fixes"
>> them, or in the sense that the underlying cause for the warnings is not
>> fixed and someone might want to look into them at a later time?
>>
>> FWIW, I hadn't considered the latter (it doesn't seem too hard to find
>> such warnings anyway; every time I try bootstrap-O3, which is not very
>> often, I get a few of those), and the former didn't seem enough of a
>> reason to have bug reports created.
>
> The latter; false positive warnings like this are bugs that should be fixed.
For the -O1 stuff the lower optimization level disables the optimizers 
that would identify and optimize away the infeasible path.

For -O3 cases with unrolling, the unroller can create oob array 
references with a guard.  Our ability to exploit the guard isn't as 
strong as it should be -- we've got BZs for this already.

For the -O3 these are cases where the inliner exposes the infeasible 
path, but we're not currently able to analyze the infeasible path.

One could argue that any initializer we add ought to avoid a false 
positive for -O2 or -O3 ought to be marked and a bug opened.  But I 
didn't feel it was worth pushing for that level of rigor since at most 
these are missed optimizations.  But it would certainly be nice.

jeff
David Malcolm Jan. 4, 2017, 9:41 p.m. UTC | #6
On Wed, 2017-01-04 at 09:04 -0700, Jeff Law wrote:
> On 01/04/2017 06:42 AM, Jason Merrill wrote:
> > On Wed, Jan 4, 2017 at 8:13 AM, Alexandre Oliva <aoliva@redhat.com>
> > wrote:
> > > On Jan  3, 2017, Jason Merrill <jason@redhat.com> wrote:
> > > 
> > > > Are there bugzillas for these false positive warnings?
> > > 
> > > I don't think so.
> > > 
> > > Did you mean in the sense that the patch silences them and thus
> > > "fixes"
> > > them, or in the sense that the underlying cause for the warnings
> > > is not
> > > fixed and someone might want to look into them at a later time?
> > > 
> > > FWIW, I hadn't considered the latter (it doesn't seem too hard to
> > > find
> > > such warnings anyway; every time I try bootstrap-O3, which is not
> > > very
> > > often, I get a few of those), and the former didn't seem enough
> > > of a
> > > reason to have bug reports created.
> > 
> > The latter; false positive warnings like this are bugs that should
> > be fixed.
> For the -O1 stuff the lower optimization level disables the
> optimizers 
> that would identify and optimize away the infeasible path.
> 
> For -O3 cases with unrolling, the unroller can create oob array 
> references with a guard.  Our ability to exploit the guard isn't as 
> strong as it should be -- we've got BZs for this already.
> 
> For the -O3 these are cases where the inliner exposes the infeasible 
> path, but we're not currently able to analyze the infeasible path.
> 
> One could argue that any initializer we add ought to avoid a false 
> positive for -O2 or -O3 ought to be marked and a bug opened.  But I 
> didn't feel it was worth pushing for that level of rigor since at
> most 
> these are missed optimizations.  But it would certainly be nice.

FWIW I've filed PR middle-end/78993 about the input.c false positive
that Alex's patch addresses; I *think* it's a case where the inliner
has exposed an infeasible path (but I could be wrong).
diff mbox

Patch

diff --git a/gcc/input.c b/gcc/input.c
index dcb5101..a478873 100644
--- a/gcc/input.c
+++ b/gcc/input.c
@@ -2168,7 +2168,7 @@  assert_char_at_range (const location &loc,
   cpp_reader *pfile = test.m_parser;
   string_concat_db *concats = &test.m_concats;
 
-  source_range actual_range;
+  source_range actual_range = source_range();
   const char *err
     = get_source_range_for_char (pfile, concats, strloc, type, idx,
 				 &actual_range);