Patchwork speed up ifcvt:cond_move_convert_if_block

login
register
mail settings
Submitter Steven Bosscher
Date Aug. 15, 2012, 11:11 p.m.
Message ID <CABu31nODe7wv626WKELD0eAiDxEV-OTq8ijVy3ftA8KuAdwwYw@mail.gmail.com>
Download mbox | patch
Permalink /patch/177879/
State New
Headers show

Comments

Steven Bosscher - Aug. 15, 2012, 11:11 p.m.
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:



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
Richard Guenther - Aug. 16, 2012, 10:06 a.m.
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

Patch

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;