[PATCH] D53534: [hot-cold-split] Name split functions with ".cold" suffix

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: lib/Transforms/Utils/CodeExtractor.cpp:678
+      Suffix.empty() ? oldFunction->getName() + "_" + header->getName()
+                     : oldFunction->getName() + Suffix,
+      M);
----------------
tejohnson wrote:
> sebpop wrote:
> > tejohnson wrote:
> > > sebpop wrote:
> > > > In the case of more than one region extracted from a function, this code may create functions with identical names which would create link problems.
> > > > 
> > > Will the hot cold split pass extract more than one cold function from a single function?
> > Yes with https://reviews.llvm.org/D53588
> Ok I see. It would make sense then to append a unique id, which could be the header name. Also we may in general want to change the "_" here to a "." so that the demangler handles it appropriately, regardless of whether we change the suffix after that deliminator.
> 
> I did see in my internal project that a lot of the cold split funcs actually ended with "_" without any header label - I need to go back and investigate - do we have empty header names in some cases? Because if so, with splitting into multiple funcs we will also end up with name collisions. Let me take another look...
> 
> 
When the IR is generated by a release built clang, the bb labels are empty. So that will cause issues with your D53588 patch, since all outlined funcs will be named original name + "_". So they need to use some other id when the name is empty. Perhaps the client of CodeExtractor needs to supply a unique id (for hot cold split you could just enumerate the split out functions for each original function and use that as a unique id).


Repository:
  rL LLVM

https://reviews.llvm.org/D53534