[PATCH] D105349: [llvm][Inline] Add interface to return cost-benefit stuff

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added inline comments.


================
Comment at: llvm/include/llvm/Analysis/InlineCost.h:63
+
+  APInt Cost;
+  APInt Benefit;
----------------
taolq wrote:
> mtrofin wrote:
> > const, both here and below.
> I have been trying to use const, but it still doesn't go well. It still needs an assignment operation. 
> In `ReplayInlineAdvisor::getAdviceImpl`
> 
> ```
>   Optional<InlineCost> InlineRecommended = None;
>   if (Iter != InlineSitesFromRemarks.end()) {
>     InlineRecommended = llvm::InlineCost::getAlways("found in replay");
>   }
> ```
> 
> By the way, it is always passed by value, just like the Cost or Threshold. 
> So I think it is safe to use w/o const, right?
It's not about safety, it's readability and maintainability: avoiding folks later having to chase down fields to figure out where they are set.

A way to address this would be to make CostBenefitPair a class, expose the fields through const accessors, and then you can have the default assignment operator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105349