[PATCH] D15243: [PGO]: Do not use invalid Char in instrumentation variable names

[Xinliang David Li via llvm-commits]

On Fri, Dec 4, 2015 at 7:50 PM, Sean Silva <chisophugis at gmail.com> wrote:

> silvas added a subscriber: silvas.
> silvas added a comment.
>
> I would prefer not to silently break compatibility. These things tend to
> come back and bite. In particular, `static` functions are quite common, and
> we are unlikely to "do the right thing" with just this patch (actually, I
> know that this will affect the intrinsics in our built-in headers, so most
> code is likely to be affected). Do we have a versioning mechanism available
> so that we can produce a proper diagnostic and reject the file?
>
>
For this particular case, there is actually a way to make the change
backward compatible.


> Personally, I am not strongly opposed to changing the format. We are still
> rolling out PGO to our customers and haven't produced explicit
> documentation on this aspect, but my intention is that we will document to
> our users that we do not guarantee any compatibility of the profraw or
> profdata files across releases.



My understanding is that we don't promise profraw compatibility across
releases.  It makes sense to keep the compatibility for indexed format --
we even have regression test to guarantee v1 format still works.  In fact,
we now have mechanism set up such that making compatible changes for
indexed format does not require too much effort for most of the cases.




> So in principle I have no concerns with breaking old profraw or profdata
> files as long as we cleanly reject them (besides a slight apprehension
> since I often am comparing compilers across releases and it is convenient
> to not have to have a separate llvm-profdata for each release).
>
>
This is a valid use case -- assuming program sources don't change. If they
changed, keeping old profile data do not make sense anymore.


> So I will defer to Justin about whether we should break compatibility over
> this aspect and if so, how we should handle the transition.
>
>
No worries -- this one can be compatible.

thanks,

David


>
> http://reviews.llvm.org/D15243
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20151204/2585c1f5/attachment.html>