Re: [PATCH] Fix lots of uninitialized memory uses in sched_analyze_reg

2013-03-05 Thread Richard Biener 
On Mon, Mar 4, 2013 at 10:17 PM, Jakub Jelinek ja...@redhat.com wrote:
 Hi!

 Something that again hits lots of testcases during valgrind checking
 bootstrap.  init_alias_analysis apparently does
   vec_safe_grow_cleared (reg_known_value, maxreg - FIRST_PSEUDO_REGISTER);
   reg_known_equiv_p = sbitmap_alloc (maxreg - FIRST_PSEUDO_REGISTER);
 but doesn't bitmap_clear (reg_known_equiv_p), perhaps as an optimization?
 If set_reg_known_value is called (and not to the reg itself),
 set_reg_known_equiv_p is called too though.
 Right now get_reg_known_equiv_p is only called in one place, and we are only
 interested in MEM_P known values there, so the following works fine.
 Though perhaps if in the future we use the reg_known_equiv_p bitmap more,
 we should bitmap_clear (reg_known_equiv_p) it instead.
 Bootstrapped/regtested on x86_64-linux and i686-linux.

 Ok for trunk (or do you prefer to slow down init_alias_analysis and just
 clear the bitmap)?

Looks ok, also clear the sbitmap as of stevens comment.

Thanks,
Richard.

 2013-03-04  Jakub Jelinek  ja...@redhat.com

 * sched-deps.c (sched_analyze_reg): Only call get_reg_known_equiv_p
 if get_reg_known_value returned non-NULL.

 --- gcc/sched-deps.c.jj 2013-03-04 12:21:09.0 +0100
 +++ gcc/sched-deps.c2013-03-04 17:29:03.478944157 +0100
 @@ -2351,10 +2351,10 @@ sched_analyze_reg (struct deps_desc *dep
/* Pseudos that are REG_EQUIV to something may be replaced
  by that during reloading.  We need only add dependencies for
 the address in the REG_EQUIV note.  */
 -  if (!reload_completed  get_reg_known_equiv_p (regno))
 +  if (!reload_completed)
 {
   rtx t = get_reg_known_value (regno);
 - if (MEM_P (t))
 + if (t  MEM_P (t)  get_reg_known_equiv_p (regno))
 sched_analyze_2 (deps, XEXP (t, 0), insn);
 }


 Jakub

Re: [PATCH] Fix lots of uninitialized memory uses in sched_analyze_reg

2013-03-05 Thread Vladimir Makarov 

On 13-03-04 4:17 PM, Jakub Jelinek wrote:

Hi!

Something that again hits lots of testcases during valgrind checking
bootstrap.  init_alias_analysis apparently does
   vec_safe_grow_cleared (reg_known_value, maxreg - FIRST_PSEUDO_REGISTER);
   reg_known_equiv_p = sbitmap_alloc (maxreg - FIRST_PSEUDO_REGISTER);
but doesn't bitmap_clear (reg_known_equiv_p), perhaps as an optimization?
Sorry, I don't know current state of alias.c well to say something 
definite about this.  But I believe it should be cleared.

If set_reg_known_value is called (and not to the reg itself),
set_reg_known_equiv_p is called too though.
Right now get_reg_known_equiv_p is only called in one place, and we are only
interested in MEM_P known values there, so the following works fine.
Though perhaps if in the future we use the reg_known_equiv_p bitmap more,
we should bitmap_clear (reg_known_equiv_p) it instead.
Bootstrapped/regtested on x86_64-linux and i686-linux.

Ok for trunk (or do you prefer to slow down init_alias_analysis and just
clear the bitmap)?
I don't see any harm from your patch but I guess it should be fixed by 
clearing reg_know_equiv_p.  I think you need Steven's opinion on this as 
he is an author of the code.


2013-03-04  Jakub Jelinek  ja...@redhat.com

* sched-deps.c (sched_analyze_reg): Only call get_reg_known_equiv_p
if get_reg_known_value returned non-NULL.

--- gcc/sched-deps.c.jj 2013-03-04 12:21:09.0 +0100
+++ gcc/sched-deps.c2013-03-04 17:29:03.478944157 +0100
@@ -2351,10 +2351,10 @@ sched_analyze_reg (struct deps_desc *dep
/* Pseudos that are REG_EQUIV to something may be replaced
 by that during reloading.  We need only add dependencies for
the address in the REG_EQUIV note.  */
-  if (!reload_completed  get_reg_known_equiv_p (regno))
+  if (!reload_completed)
{
  rtx t = get_reg_known_value (regno);
- if (MEM_P (t))
+ if (t  MEM_P (t)  get_reg_known_equiv_p (regno))
sched_analyze_2 (deps, XEXP (t, 0), insn);
}

Re: [PATCH] Fix lots of uninitialized memory uses in sched_analyze_reg

2013-03-05 Thread Jakub Jelinek 
On Tue, Mar 05, 2013 at 11:58:09PM -0500, Vladimir Makarov wrote:
 I don't see any harm from your patch but I guess it should be fixed
 by clearing reg_know_equiv_p.  I think you need Steven's opinion on
 this as he is an author of the code.

Yeah, I've already committed the clearing of the sbitmap in alias.c
instead of this sched-deps.c patch, which doesn't make sense after the
alias.c change.

Jakub

[PATCH] Fix lots of uninitialized memory uses in sched_analyze_reg

2013-03-04 Thread Jakub Jelinek 
Hi!

Something that again hits lots of testcases during valgrind checking
bootstrap.  init_alias_analysis apparently does
  vec_safe_grow_cleared (reg_known_value, maxreg - FIRST_PSEUDO_REGISTER);
  reg_known_equiv_p = sbitmap_alloc (maxreg - FIRST_PSEUDO_REGISTER);
but doesn't bitmap_clear (reg_known_equiv_p), perhaps as an optimization?
If set_reg_known_value is called (and not to the reg itself),
set_reg_known_equiv_p is called too though.
Right now get_reg_known_equiv_p is only called in one place, and we are only
interested in MEM_P known values there, so the following works fine.
Though perhaps if in the future we use the reg_known_equiv_p bitmap more,
we should bitmap_clear (reg_known_equiv_p) it instead.
Bootstrapped/regtested on x86_64-linux and i686-linux.

Ok for trunk (or do you prefer to slow down init_alias_analysis and just
clear the bitmap)?

2013-03-04  Jakub Jelinek  ja...@redhat.com

* sched-deps.c (sched_analyze_reg): Only call get_reg_known_equiv_p
if get_reg_known_value returned non-NULL.

--- gcc/sched-deps.c.jj 2013-03-04 12:21:09.0 +0100
+++ gcc/sched-deps.c2013-03-04 17:29:03.478944157 +0100
@@ -2351,10 +2351,10 @@ sched_analyze_reg (struct deps_desc *dep
   /* Pseudos that are REG_EQUIV to something may be replaced
 by that during reloading.  We need only add dependencies for
the address in the REG_EQUIV note.  */
-  if (!reload_completed  get_reg_known_equiv_p (regno))
+  if (!reload_completed)
{
  rtx t = get_reg_known_value (regno);
- if (MEM_P (t))
+ if (t  MEM_P (t)  get_reg_known_equiv_p (regno))
sched_analyze_2 (deps, XEXP (t, 0), insn);
}
 

Jakub

Re: [PATCH] Fix lots of uninitialized memory uses in sched_analyze_reg

2013-03-04 Thread Steven Bosscher 
On Mon, Mar 4, 2013 at 10:17 PM, Jakub Jelinek wrote:
 Hi!

 Something that again hits lots of testcases during valgrind checking
 bootstrap.  init_alias_analysis apparently does
   vec_safe_grow_cleared (reg_known_value, maxreg - FIRST_PSEUDO_REGISTER);
   reg_known_equiv_p = sbitmap_alloc (maxreg - FIRST_PSEUDO_REGISTER);
 but doesn't bitmap_clear (reg_known_equiv_p), perhaps as an optimization?

No, an incorrect replacement with sbitmap. We used to have:

  reg_known_equiv_p = XCNEWVEC (bool, reg_known_value_size);

I'm probably to blame for this one, actually :-)

Ciao!
Steven

[PATCH] Fix lots of uninitialized memory uses in sched_analyze_reg (take 2)

2013-03-04 Thread Jakub Jelinek 
On Mon, Mar 04, 2013 at 11:17:41PM +0100, Steven Bosscher wrote:
 On Mon, Mar 4, 2013 at 10:17 PM, Jakub Jelinek wrote:
  Something that again hits lots of testcases during valgrind checking
  bootstrap.  init_alias_analysis apparently does
vec_safe_grow_cleared (reg_known_value, maxreg - FIRST_PSEUDO_REGISTER);
reg_known_equiv_p = sbitmap_alloc (maxreg - FIRST_PSEUDO_REGISTER);
  but doesn't bitmap_clear (reg_known_equiv_p), perhaps as an optimization?
 
 No, an incorrect replacement with sbitmap. We used to have:
 
   reg_known_equiv_p = XCNEWVEC (bool, reg_known_value_size);
 
 I'm probably to blame for this one, actually :-)

This works too on the testcase (e.g. gcc.dg/54455.c).

Will bootstrap/regtest it overnight.

2013-03-04  Jakub Jelinek  ja...@redhat.com

* alias.c (init_alias_analysis): Clear reg_known_equiv_p bitmap.

--- gcc/alias.c.jj  2013-01-18 12:49:31.0 +0100
+++ gcc/alias.c 2013-03-04 23:22:38.865751120 +0100
@@ -2812,6 +2812,7 @@ init_alias_analysis (void)
 
   vec_safe_grow_cleared (reg_known_value, maxreg - FIRST_PSEUDO_REGISTER);
   reg_known_equiv_p = sbitmap_alloc (maxreg - FIRST_PSEUDO_REGISTER);
+  bitmap_clear (reg_known_equiv_p);
 
   /* If we have memory allocated from the previous run, use it.  */
   if (old_reg_base_value)


Jakub