[PATCH] D110271: [llvm-profgen] Fix an out-of-range error during unwinding

[Lei Wang via Phabricator via llvm-commits]

wlei added inline comments.


================
Comment at: llvm/tools/llvm-profgen/PerfReader.cpp:81
+      if (!SameInlinee) {
+        State.switchToFrame(End);
         State.CurrentLeafFrame->recordRangeCount(PrevIP, End, Repeat);
----------------
wlei wrote:
> wenlei wrote:
> > wlei wrote:
> > > wenlei wrote:
> > > > This is a change beyond refactoring? Before, we use last address of old frame as frame address, but now it's going to be the first address of old frame. 
> > > > 
> > > > `State.switchToFrame(LeafAddr);`
> > > > ->
> > > > `State.switchToFrame(End);`
> > > > 
> > > > Or do did you actually meant `State.switchToFrame(PrevIP);`?
> > > > 
> > > > 
> > > It won't affect the result. I think any address in [PrevIP, End] is good for the leaf frame because we will trim the location info of the last frame. Using `End` is because `End` is also used out of the loop(just for readability).
> > Ok, so it changes what's used in the frame trie, but as long as we use a consistent address to represent frame, changes there won't affect output, correct?
> Sorry, my fault, it should be the last address of the old frame, otherwise if the last address is a call, it will be recorded in the wrong callsite. Thanks for correcting me! 
OK, I double checked the call address won't use the address from linear unwinder, it use the LBR address. So changing to `End` should be fine. Anyway I choose use the last address which align to the program execution logic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110271