[PATCH] D76248: Fix a bug in the inliner that causes subsequent double inlining

[Hongtao Yu via Phabricator via llvm-commits]

hoyFB added a comment.

In D76248#1935246 <https://reviews.llvm.org/D76248#1935246>, @davidxl wrote:

> Thanks for the explanation.
>
> The inliner does bottom up inlining -- after inlining each site for a node, a worklist based TopDown Inlining is attempted (in breadth first order) -- i.e., the new callsites from the the cloned inline instance will be pushed to the queue. The candidate callsites must be originated from the callee, not a reference to a callsite in the caller due to simplification.  I think the better fix is at the queue update :
>
>   if (!InlineInfo.InlinedCalls.empty()) {
>      // Create a new inline history entry for this, so that we remember
>      // that these new callsites came about due to inlining Callee.
>  
>      int NewHistoryID = InlineHistory.size();
>      InlineHistory.push_back(std::make_pair(Callee, InlineHistoryID));
>   
>      for (Value *Ptr : InlineInfo.InlinedCalls)
>        CallSites.push_back(std::make_pair(CallSite(Ptr), NewHistoryID));
>    }
>   
>
> In otherwords, if the callsite is already in the queue, do not attempt to insert it into it.


Thanks for the pointers! Yes, I did try your fix first before making the current fix. Not queueing the same callisite twice fixes the doubling inlining, but another issue that the callgraph edge corresponding to the inlined `wrapper` call in `top` still is based on the callsite `top->gen`. When later on `top->gen` is inlined and removed, the `wrapper` call edge would AV. I think the culprit is that the `top->gen` callsite is shared by two call edges `top->gen` and `top->wrapper`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76248