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

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: lib/Analysis/InlineCost.cpp:327
+  Cost += LoadEliminationCost;
+  LoadEliminationCostSavings = 0;
+  EnableLoadElimination = false;
----------------
haicheng wrote:
> mehdi_amini wrote:
> > haicheng wrote:
> > > 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.
> > But calling multiple times disableLoadElimination will increase the Cost which does not seem right?
> disableLoadElimination() should be called only once.  It is guarded by the flag EnableLoadElimination.  I missed one check in disableSROA() before calling disableLoadElimination() and now I fix it.  Thank you.
It seems fragile: I'd expect the API itself to be idempotent. I'm fine with guarding it on EnableLoadElimination but the guard should be in the method itself.


Repository:
  rL LLVM

https://reviews.llvm.org/D33946