[PATCH] D108433: [CSSPGO] split context string - compiler changes

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:575-582
+    SampleCallSiteType ThisLeaf = ThisContext.back();
+    SampleCallSiteType ThatLeaf = ThatContext.back();
+    if (!ThisLeaf.hasLineLocation()) {
+      ThisLeaf.dropLineLocation();
+      ThatLeaf.dropLineLocation();
     }
+    if (ThisLeaf != ThatLeaf)
----------------
hoy wrote:
> wenlei wrote:
> > hoy wrote:
> > > wenlei wrote:
> > > > This sequence of copy, edit and compare could be simplified?
> > > > 
> > > > ```
> > > > if (ThisContext.back().CallerName != ThatContext.back().CallerName)
> > > >   return false;
> > > > ```
> > > > 
> > > > Even better, would it be possible to canonicalize context, so location of leaf frame is always (0, 0)?
> > > The API can be used to compare `A:1 @ B` with `A:1 @ B:2 @ C`, where we have to drop the line number of `B:2`.
> > > 
> > > The location of leaf frame was (0,0) previously, now I'm changing it to (-1,0). But the `that` context can still have a valid line number like `B:2` where we have to drop the line number for comparision.
> > Ok, if we have to ignore the location part of B, can we simplified the copy, edit and compare sequence as suggested above still? 
> > 
> > (maybe i'm missing something, but it looks to me that you answered the 2nd question only?) 
> Do you mean something like below?
> 
> 
> ```
> if (ThisContext.back().CallerName != ThatContext.back().CallerName)
>   return false;
> 
> if (ThisContext.back().hasLineLocation()) 
>    return ThisContext.back().linelocation == ThatContext.back().linelocation 
>    
> return true;
> ```
`return true` --> `return ThisContext.drop_back() == ThatContext.drop_back()`


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108433/new/

https://reviews.llvm.org/D108433