[NFC][InstrProf]Factor out getCanonicalName to compute the canonical name given a pgo name. #81547
Conversation
mingmingl-llvm
requested review from
teresajohnson,
david-xl,
modiking and
snehasish
| // Returns the canonial name of the given PGOName. In a canonical name, all | ||
| // suffixes that begins with "." except ".__uniq." are stripped. | ||
| // FIXME: Unify this with `FunctionSamples::getCanonicalFnName`. And both | ||
| // should probably preserve `.specialized.<NSpecs>` suffix now that |
There was a problem hiding this comment.
It isn't clear to me why we would want to preserve .specialized, since presumably those specialized copies are created after PGO annotation / matching.
There was a problem hiding this comment.
ah you are correct. I initially thought stripping .specialized.<NSpecs> from function names gives in-accurate profile matching (maps names corresponding to different functions to one &F), but the specialized copies doesn't exist at profile annotation. I'm not aware of other places where the profiles for specialized copies (if the specialized copies exist in the instrumented or sampled binary) are handled, but I'll remove the comment.
…s doesn't exist in the IR at profile annotation time
|
I read this PR again and made a change to use |
llvm/lib/ProfileData/InstrProf.cpp
Outdated
| SmallVector<StringRef, 2> Names; | ||
| Names.push_back(PGOFuncName); | ||
| StringRef CanonicalFuncName = getCanonicalName(PGOFuncName); | ||
| if (CanonicalFuncName != PGOFuncName) |
There was a problem hiding this comment.
Since you are doing this check explicitly here, rather than add to a vector and iterating it, suggest putting the handling currently within the loop body into a lambda and simply calling it twice (with the second call guarded by this if).
llvm/lib/ProfileData/InstrProf.cpp
Outdated
| if (CanonicalFuncName == PGOFuncName) | ||
| return Error::success(); | ||
|
|
||
| return mapName(CanonicalFuncName); |
There was a problem hiding this comment.
slight preference for doing this guarded by if != and return Error::success in the default case.
but otherwise lgtm
This is a PR into llvm:main branch as it simplifies
addFuncWithName.InstrProf::addFuncWithNameto call the newly addedgetCanonicalName.