[PATCH] D33946: [InlineCost] Find identical loads in the callee

Haicheng Wu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 13 06:17:43 PDT 2017


haicheng added a comment.

Hi Mehdi,

I can remove LoadEliminationCostSavings if you think it is better.

Haicheng



================
Comment at: lib/Analysis/InlineCost.cpp:265
   unsigned SROACostSavingsLost;
+  unsigned CSELoadCostSavings;
 
----------------
mehdi_amini wrote:
> haicheng wrote:
> > mehdi_amini wrote:
> > > chandlerc wrote:
> > > > Similarly, I would call this `LoadCSECostSavings`.
> > > Why do we have both `LoadEliminationCostSavings` and `LoadEliminationCost` ?
> > `LoadEliminationCost` is used in the cost calculation, and  `LoadEliminationCostSavings` is used for stats printing.  This is the same as `SROACostSavings` and `SROAArgCosts`.
> I'm still not making sense of the difference between the two: as far as I can tell they are zero-initialized in the same ctor and gets incremented the same way?
Your understanding is correct.  If load elimination is not disabled, they should have the same value.  LoadEliminationCostSavings is just used in stats printing.  

Eventually, every load address needs to have its own LoadEliminationCost as written in the FIXME no.1.  At that time, LoadEliminationCostSavings is the sum of all LoadEliminationCost.


================
Comment at: lib/Analysis/InlineCost.cpp:327
+  Cost += LoadEliminationCost;
+  LoadEliminationCostSavings = 0;
+  EnableLoadElimination = false;
----------------
mehdi_amini wrote:
> It seems to me that it is `LoadEliminationCost` that needs to be set to 0 instead `LoadEliminationCostSavings`?
When load elimination is disabled, LoadEliminationCostSavings needs to be set to 0 so that we can print the correct value.  LoadEliminationCost is not used any more after LoadElimination is disabled.


Repository:
  rL LLVM

https://reviews.llvm.org/D33946





More information about the llvm-commits mailing list