| Message ID | CABu31nODe7wv626WKELD0eAiDxEV-OTq8ijVy3ftA8KuAdwwYw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Aug 16, 2012 at 1:11 AM, Steven Bosscher <stevenb.gcc@gmail.com> wrote: > On Mon, Aug 6, 2012 at 1:27 PM, Paolo Bonzini <bonzini@gnu.org> wrote: >>> 2. sparseset has the same problem of memory clearing (for valgrind, >>> see sparseset_alloc). >> >> ... only the sparse array needs this clearing, but currently we do it >> for both. > > And according to the fat comment before the xcalloc, it's not even > really needed. Why can't we just tell valgrind to treat the memory as > defined, like in this patchlet: Indeed ... I suppose ok if it bootstraps / tests w/o valgrind checking and if a cc1 builds with valgrind checking. Thanks, Richard. > Index: sparseset.c > =================================================================== > --- sparseset.c (revision 190418) > +++ sparseset.c (working copy) > @@ -30,12 +30,14 @@ sparseset_alloc (SPARSESET_ELT_TYPE n_elms) > unsigned int n_bytes = sizeof (struct sparseset_def) > + ((n_elms - 1) * 2 * sizeof (SPARSESET_ELT_TYPE)); > > - /* We use xcalloc rather than xmalloc to silence some valgrind uninitialized > + sparseset set = XNEWVAR(sparseset, n_bytes); > + > + /* Mark the sparseset as defined to silence some valgrind uninitialized > read errors when accessing set->sparse[n] when "n" is not, and never has > been, in the set. These uninitialized reads are expected, by design and > - harmless. If this turns into a performance problem due to some future > - additional users of sparseset, we can revisit this decision. */ > - sparseset set = (sparseset) xcalloc (1, n_bytes); > + harmless. */ > + VALGRIND_DISCARD (VALGRIND_MAKE_MEM_DEFINED (set, n_bytes)); > + > set->dense = &(set->elms[0]); > set->sparse = &(set->elms[n_elms]); > set->size = n_elms; > > > I have never built GCC with valgrind checking, so I don't know if this > is correct. But there has to be a better solution than calling > xcalloc... > > Ciao! > Steven
Index: sparseset.c =================================================================== --- sparseset.c (revision 190418) +++ sparseset.c (working copy) @@ -30,12 +30,14 @@ sparseset_alloc (SPARSESET_ELT_TYPE n_elms) unsigned int n_bytes = sizeof (struct sparseset_def) + ((n_elms - 1) * 2 * sizeof (SPARSESET_ELT_TYPE)); - /* We use xcalloc rather than xmalloc to silence some valgrind uninitialized + sparseset set = XNEWVAR(sparseset, n_bytes); + + /* Mark the sparseset as defined to silence some valgrind uninitialized read errors when accessing set->sparse[n] when "n" is not, and never has been, in the set. These uninitialized reads are expected, by design and - harmless. If this turns into a performance problem due to some future - additional users of sparseset, we can revisit this decision. */ - sparseset set = (sparseset) xcalloc (1, n_bytes); + harmless. */ + VALGRIND_DISCARD (VALGRIND_MAKE_MEM_DEFINED (set, n_bytes)); + set->dense = &(set->elms[0]); set->sparse = &(set->elms[n_elms]); set->size = n_elms;