Minor fixes in Loop Strength Reduction

[evstupac]

Summary

The patch contains the following minor fixes:

1. Function renaming: from specific "Condition Instruction" to general "Instruction".
2. Debug print: form address print to Instruction dump.
3. Set count of first register in best registers search.

[anna]

The second and third changes look good. Is there a reason for the first one?

[evstupac]

The second and third changes look good. Is there a reason for the first one?

Anna, thanks for taking a look.
Regarding 1st change.
It just widen the function usability.
Function "FindIVUserForCond" is general and could be used not only for "Conditions". It can do the same search for any Instruction.
I want to reuse it in one of my future commits for something more than ICmpInst.

[anna]

Evgeny ,
This looks good to me, but I'll leave it to others to explicitly LGTM.

You can add Quentin as well as reviewer (he's the new code owner of LSR).

[hfinkel]

lib/Transforms/Scalar/LoopStrengthReduce.cpp

2818 ↗

(On Diff #78605)

This debugging-statement change LGTM; please commit this separately.

4149 ↗

(On Diff #78605)

Do you have a test case for this change?

If/When you commit this, it should be separate from the other changes.

[evstupac]

lib/Transforms/Scalar/LoopStrengthReduce.cpp

4149 ↗

(On Diff #78605)

No. I don't. I can try to create.
However, it looks like a misprint in MAX search algorithm. We are always missing the case when first Reg in RegUses has the biggest RegUses.getUsedByIndices(...).count().

[hfinkel]

lib/Transforms/Scalar/LoopStrengthReduce.cpp

4149 ↗

(On Diff #78605)

Okay; makes sense to me.

[evstupac]

"2. Debug print: ..." committed separately.

[evstupac]

lib/Transforms/Scalar/LoopStrengthReduce.cpp

4149 ↗

(On Diff #78605)

I'm I right that I can go ahead and commit this part?
Or we still need a test case?
Wei Mi gave me a test case, so I can add it here. But I don't think that for the changes like this we need a test case in LIT tests.

[evstupac]

PING.
Is it ok to fix MAX calculation?

[qcolombet]

Hi,

LGTM.

1. Function renaming: from specific "Condition Instruction" to general "Instruction".

I would wait for that change to when we actually need it though.

Cheers,
-Quentin

[evstupac]

Committed r288278

llvm/lib/Transforms/Scalar/LoopStrengthReduce.cpp

while (EstimateSearchSpaceComplexity() >= ComplexityLimit) {
 
     // Pick the register which is used by the most LSRUses, which is likely
     // to be a good reuse register candidate.
     const SCEV *Best = nullptr;
     unsigned BestNum = 0;
     for (const SCEV *Reg : RegUses) {
       if (Taken.count(Reg))
         continue;
-      if (!Best)
+      if (!Best) {
         Best = Reg;
-      else {
+        BestNum = RegUses.getUsedByIndices(Reg).count();
+      } else {
         unsigned Count = RegUses.getUsedByIndices(Reg).count();
         if (Count > BestNum) {
           Best = Reg;
           BestNum = Count;
         }
       }
     }