[PATCH] D51247: Support for remapping profile data, for instrumentation-based profiling.

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: lib/ProfileData/InstrProfReader.cpp:606
+template<typename HashTableImpl>
+class llvm::InstrProfReaderItaniumRemapper : public InstrProfReaderIndexBase {
+public:
----------------
rsmith wrote:
> davidxl wrote:
> > I suggest moving this remapping support code out of the ProfData core library into llvm-profdata.cpp where an user option (for llvm_profdata) is provided so that the profile data with old naming schemes can be converted to new format:
> > 
> > something like:
> > 
> > llvm_profdata merge -o <new_format_prof_data> -remapping_file=<>   <old_profile_data>
> > 
> > 
> > By so doing, compiler does not need to be made aware of it. 
> This approach doesn't really support that workflow: we don't have the ability to form remapped mangled names at all, only the ability to tell if two names are the same modulo a set of equivalences. (As noted on D51248, if we took the "remangling" path we'd add significant complexity and also put a significant maintenance burden on ourselves to keep a remangler up to date with extensions to the demangler, which is not necessary in this equivalence-based approach.)
> 
> We could do something similar where the tool is given the old file, the remapping file, and a list of symbols in the new program, but that seems cumbersome to use (you'd need additional steps after compilation to extract the list of symbols and remap the profile data, and then you'd need to rerun the compilation with the updated profile data). The idea of this approach is that you merely need to add a remapping file and one flag to your compilations, and they will transparently be able to use profiling data from either before or after a symbol renaming with no change to the steps in the build process.
Are there libraries to do name mangling and demangling? If that is the case, linking them into the profile tool has a lot of advantage over the approach that requires compiler awareness:

1) it can handle different 'old' C++ ABIs
2) it can handle indirect call targets -- which is very important for performance
3) it does not require any changes in the compiler, no new options are needed

I think the most important point is 2) -- we may need to do it because of the performance reasons anyway.

If we can not do mangling/remagnalling, I don't see the option to require passing symbol list too bad either.  Note that llvm-cov tool also requires user to pass the execuable path.  We can do the same here -- pass a newly compiled executable to the tool so that the tool can extract the newly named symbols.



Repository:
  rL LLVM

https://reviews.llvm.org/D51247