[PATCH] D55877: [LIR] Add CTTZ support part2

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1511
+  // Find CntPhi as the phi SCEV that is {base,+,1}, base must be constant
+  for (auto I = LoopEntry->begin(),
+            E = LoopEntry->getFirstNonPHI()->getIterator(); I != E; ++I) {
----------------
tabloid.adroit wrote:
> craig.topper wrote:
> > Why are we doing this a different way than detectShiftUntilZeroIdiom? And why does the base need to be constant?
> I thought they are an equivalent way to detect CntPhi and IMHO more clear to read than the way used by detectShiftUntilZeroIdiom.
> 
> Making the base constant is unintentional. Updated patch to reflect that.
I'm sure they are equivalent. I just think we should try to have consistent implementations for the same thing in the same area of the compiler.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55877/new/

https://reviews.llvm.org/D55877