Fix bootstrap failure in vect_analyze_loop

Message ID 1529975083-13811-1-git-send-email-dmalcolm@redhat.com
State New
Headers show
Series
  • Fix bootstrap failure in vect_analyze_loop
Related show

Commit Message

David Malcolm June 26, 2018, 1:04 a.m.
I ran into this bootstrap failure (with r262092):

../../../src/gcc/tree-vect-loop.c: In function ‘_loop_vec_info* vect_analyze_loop(loop*, loop_vec_info, vec_info_shared*)’:
../../../src/gcc/tree-vect-loop.c:1946:25: error: ‘n_stmts’ may be used uninitialized in this function [-Werror=maybe-uninitialize ]
   ok = vect_analyze_slp (loop_vinfo, *n_stmts);
        ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~
../../../src/gcc/tree-vect-loop.c:2318:12: note: ‘n_stmts’ was declared here
   unsigned n_stmts;
            ^~~~~~~
cc1plus: all warnings being treated as errors

It looks like it's inlining vect_analyze_loop_2 into vect_analyze_loop,
passing &n_stmts in by pointer.

Normally, vect_get_datarefs_in_loop writes:
  *n_stmts = 0;
when
  if (!LOOP_VINFO_DATAREFS (loop_vinfo).exists ())
but not in the "else" path, and then, after lots of complex logic:

  ok = vect_analyze_slp (loop_vinfo, *n_stmts);

it uses the value in vect_analyze_loop_2, passed in via &n_stmts.

So it's not at all clear to me (or the compiler) that the value is
initialized in all paths, and an initialization to zero seems a
reasonable fix.

OK for trunk, assuming the bootstrap succeeds this time?

gcc/ChangeLog:
	* tree-vect-loop.c (vect_analyze_loop): Initialize n_stmts.
---
 gcc/tree-vect-loop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
1.8.5.3

Comments

Richard Biener June 26, 2018, 8:06 a.m. | #1
On Tue, Jun 26, 2018 at 2:21 AM David Malcolm <dmalcolm@redhat.com> wrote:
>

> I ran into this bootstrap failure (with r262092):

>

> ../../../src/gcc/tree-vect-loop.c: In function ‘_loop_vec_info* vect_analyze_loop(loop*, loop_vec_info, vec_info_shared*)’:

> ../../../src/gcc/tree-vect-loop.c:1946:25: error: ‘n_stmts’ may be used uninitialized in this function [-Werror=maybe-uninitialize ]

>    ok = vect_analyze_slp (loop_vinfo, *n_stmts);

>         ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~

> ../../../src/gcc/tree-vect-loop.c:2318:12: note: ‘n_stmts’ was declared here

>    unsigned n_stmts;

>             ^~~~~~~

> cc1plus: all warnings being treated as errors

>

> It looks like it's inlining vect_analyze_loop_2 into vect_analyze_loop,

> passing &n_stmts in by pointer.

>

> Normally, vect_get_datarefs_in_loop writes:

>   *n_stmts = 0;

> when

>   if (!LOOP_VINFO_DATAREFS (loop_vinfo).exists ())

> but not in the "else" path, and then, after lots of complex logic:

>

>   ok = vect_analyze_slp (loop_vinfo, *n_stmts);

>

> it uses the value in vect_analyze_loop_2, passed in via &n_stmts.

>

> So it's not at all clear to me (or the compiler) that the value is

> initialized in all paths, and an initialization to zero seems a

> reasonable fix.

>

> OK for trunk, assuming the bootstrap succeeds this time?


I already applied the very same fix, sorry for the breakage.

Richard.

> gcc/ChangeLog:

>         * tree-vect-loop.c (vect_analyze_loop): Initialize n_stmts.

> ---

>  gcc/tree-vect-loop.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c

> index dacc881..2b3ced2 100644

> --- a/gcc/tree-vect-loop.c

> +++ b/gcc/tree-vect-loop.c

> @@ -2315,7 +2315,7 @@ vect_analyze_loop (struct loop *loop, loop_vec_info orig_loop_vinfo,

>        return NULL;

>      }

>

> -  unsigned n_stmts;

> +  unsigned n_stmts = 0;

>    poly_uint64 autodetected_vector_size = 0;

>    while (1)

>      {

> --

> 1.8.5.3

>
Jeff Law June 26, 2018, 4:51 p.m. | #2
On 06/26/2018 02:06 AM, Richard Biener wrote:
> On Tue, Jun 26, 2018 at 2:21 AM David Malcolm <dmalcolm@redhat.com> wrote:

>>

>> I ran into this bootstrap failure (with r262092):

>>

>> ../../../src/gcc/tree-vect-loop.c: In function ‘_loop_vec_info* vect_analyze_loop(loop*, loop_vec_info, vec_info_shared*)’:

>> ../../../src/gcc/tree-vect-loop.c:1946:25: error: ‘n_stmts’ may be used uninitialized in this function [-Werror=maybe-uninitialize ]

>>    ok = vect_analyze_slp (loop_vinfo, *n_stmts);

>>         ~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~

>> ../../../src/gcc/tree-vect-loop.c:2318:12: note: ‘n_stmts’ was declared here

>>    unsigned n_stmts;

>>             ^~~~~~~

>> cc1plus: all warnings being treated as errors

>>

>> It looks like it's inlining vect_analyze_loop_2 into vect_analyze_loop,

>> passing &n_stmts in by pointer.

>>

>> Normally, vect_get_datarefs_in_loop writes:

>>   *n_stmts = 0;

>> when

>>   if (!LOOP_VINFO_DATAREFS (loop_vinfo).exists ())

>> but not in the "else" path, and then, after lots of complex logic:

>>

>>   ok = vect_analyze_slp (loop_vinfo, *n_stmts);

>>

>> it uses the value in vect_analyze_loop_2, passed in via &n_stmts.

>>

>> So it's not at all clear to me (or the compiler) that the value is

>> initialized in all paths, and an initialization to zero seems a

>> reasonable fix.

>>

>> OK for trunk, assuming the bootstrap succeeds this time?

> 

> I already applied the very same fix, sorry for the breakage.

And I'll confirm the tester is happy again.
jeff

Patch

diff --git a/gcc/tree-vect-loop.c b/gcc/tree-vect-loop.c
index dacc881..2b3ced2 100644
--- a/gcc/tree-vect-loop.c
+++ b/gcc/tree-vect-loop.c
@@ -2315,7 +2315,7 @@  vect_analyze_loop (struct loop *loop, loop_vec_info orig_loop_vinfo,
       return NULL;
     }
 
-  unsigned n_stmts;
+  unsigned n_stmts = 0;
   poly_uint64 autodetected_vector_size = 0;
   while (1)
     {