https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108552

--- Comment #42 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #40)
> (In reply to Jan Hubicka from comment #39)
> > I was wonering if we should not provide flag to turn all counts
> > volatile.   That way we will still have race conditions on their updates
> > (and it would be chepaer than atomic) but we won't run into such wrong
> > code issues nor large profile mismatches.
> 
> Yes, see above.  Or a mode in which we would just avoid hoisting and sinking
> the gcov vars but keep them non-volatile.  Or both.
> But I guess it would be nice to get Vlad's patch into trunk and release
> branches for now (perhaps with an extra check for startswith "__gcov" on
> DECL_NAME, so that we don't do it for the Fortran tokens).
> 
> As for the patch, just small nits, I think get_base_address returns always
> non-NULL, so it could be
>       if (tree expr = MEM_EXPR (res))
>         {
>           expr = get_base_address (expr);
>           if (VAR_P (expr)
>               && DECL_NONALIASED (expr)
>               && DECL_NAME (expr))
>             {
>               const char *name = IDENTIFIER_POINTER (DECL_NAME (expr));
>               /* Don't reread coverage counters from memory, if single
>                  update model is used in threaded code, other threads
>                  could change the counters concurrently.  See PR108552.  */
>               if (startswith (name, "__gcov"))
>                 return x;
>             }
>         }

Note that this isn't exactly reliable but a heuristic workaround since
MEM_EXPRs are optional and dropping them is valid (and done in some places).

I think if we want to avoid doing optimizations on gcov counters we should
make them volatile.  I suppose kernel folks would have a way to assess
any "catastrophic consequences" on optimization?  (I have a hard time
imagining them, sure that RMW will not allow add with memory operand,
but that's it?)

Reply via email to