[PATCH] D126300: [llvm][Inline] Refactor InlinePriority

[Kazu Hirata via Phabricator via llvm-commits]

kazu added a comment.

May I suggest expanding the summary?  You could say something like:

  ModuleInliner] Refactor InlineSizePriority and PriorityInlineOrder
  
  This patch introduces abstract base class InlinePriority to serve as
  the comparison function for the priority queue.  A derived class, such
  as SizePriority, may choose to cache the priorities for different
  functions for performance reasons.
  
  This design shields the type used for the priority away from classes
  outside InlinePriority and classes derived from it.  In turn,
  PriorityInlineOrder no longer needs to be a template class.

Note that you need to click "Edit Revision" to update the summary as Phabricator does not  automatically update the summary on `arc diff`.



================
Comment at: llvm/include/llvm/Analysis/InlineOrder.h:106-108
+    const auto OldPriority = Priorities[CB];
+    update(CB);
+    const auto NewPriority = Priorities[CB];
----------------
I'm a bit worried about the fact that we are hashing CB three times here.  Could we do something like this?

```
auto It = Priorities.find(CB);
const auto OldPriority = *It;
*It = evaluate(CB);
const auto NewPriority = *It;
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126300