[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 added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopStrengthReduce.cpp:4352
+      // shared among LSRUses, the less we increase the register number
+      // counter of the formula.
+      size_t FARegNum = 0;
----------------
sanjoy wrote:
> Can we instead take the total number of uses of registers and `FA.BaseRegs` and `FB.BaseRegs` and compare them?  That is:
> 
> ```
> int TotalUsesOfA = 0, TotalUsesOfB = 0;
> for (const SCEV *Reg : FA.BaseRegs)
>   TotalUsesOfA += RegUses.getUsedByIndices(Reg).count();
> for (const SCEV *Reg : FB.BaseRegs)
>   TotalUsesOfB += RegUses.getUsedByIndices(Reg).count();
> 
> if (TotalUsesOfA != TotalUsesOfB)
>   return TotalUsesOfA > TotalUsesOfB;
> ```
> 
> Or does it have to exactly be the expression you're using?
We don't want to choose the formula with the most sharing. What we expect is to have fewer registers. If a register is shared, we count it proportionally . That is why a shared register is better than a non-shared register.


================
Comment at: test/Transforms/LoopStrengthReduce/X86/lsr-filtering-scaledreg.ll:7
+%class.ZippyScatteredWriter = type { i8, i8*, i8* }
+ at e = local_unnamed_addr global %class.A { i8 0, i8 0, [5 x i32] zeroinitializer, i64 1, i64 0, i64 1 }, align 8
+ at f = local_unnamed_addr global %class.ZippyScatteredWriter* null, align 8
----------------
sanjoy wrote:
> Can you please clean up the names a bit here?  Perhaps using metarenamer?
> 
> Also, are both the loops necessary to show the difference in behavior?
metarenamer is a great tool. Thanks for the suggestion.

I reduce the test to some extend, but the second loop is still necessary to create a complex enough searching space.


Repository:
  rL LLVM

https://reviews.llvm.org/D34583