[PATCH] D34583: [LSR] Narrow search space by filtering non-optimal formulae with the same ScaledReg and Scale.

[Wei Mi via Phabricator via llvm-commits]

wmi marked 4 inline comments as done.
wmi added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopStrengthReduce.cpp:4318
+/// If a LSRUse has multiple formulae with the same ScaledReg and Scale.
+/// Pick the best one and delete the others.
+void LSRInstance::NarrowSearchSpaceByFilterFormulaWithSameScaledReg() {
----------------
sanjoy wrote:
> Just to be clear, this benefit here is that you can just look at the base regs and (cheaply) decide which one is better?  If so, add a line in the description about that.
The narrowing heuristic is to keep as many formulae with different Scale and ScaledReg pair as possible while narrowing the search space to be within limit. The benefit is that it is more likely to find out a better solution from a formulae set with more Scale and ScaledReg variations than picking the winner reg heuristic. Comments updated.


================
Comment at: lib/Transforms/Scalar/LoopStrengthReduce.cpp:4345
+      }
+      float FBRegNum = 0;
+      for (const SCEV *Reg : FB.BaseRegs) {
----------------
sanjoy wrote:
> It's not clear to me why you need to use a floating point reciprocal here.  Why not just count the total number of uses (as a integer) and compare that?  Or is there some more subtle metric you're tracking that requires this reciprocal logic (if so please comment on that).
Yeah I don't really need to use floating point. I change it to use integer. 


================
Comment at: lib/Transforms/Scalar/LoopStrengthReduce.cpp:4360
+      Regs.clear();
+      CostFB.RateFormula(TTI, FB, Regs, VisitedRegs, L, SE, DT, LU);
+      return CostFA.isLess(CostFB, TTI);
----------------
sanjoy wrote:
> As far as I can tell, in this situation we're making an arbitrary choice when the register use count is the same and both the formulas have the same cost (we'll just pick the second one).  Can we instead keep both the formulas when `Cost` does not give us a unambiguous signal?
We uses the similar strategy in other places, like in FilterOutUndesirableDedicatedRegisters at the place: CostF.isLess(CostBest, TTI)). I think the key point here is to keep the formula set small while providing more induction variable choices for LSR solver, so making an arbitrary choice to reduce the formula set may not be too bad. 


Repository:
  rL LLVM

https://reviews.llvm.org/D34583