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

[Yuanfang Chen via Phabricator via llvm-commits]

tabloid.adroit marked 3 inline comments as done.
tabloid.adroit 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) {
----------------
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.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1578
+  for (Use &U : VarX->uses()) {
+    auto *UI = dyn_cast<Instruction>(U.getUser());
+    if (!UI)
----------------
craig.topper wrote:
> Why can't we limit to just checking the preheader condition like the shiftUntilZero? Do you have more complex real world examples you're trying to handle?
Other than checking the input is non-zero, a naive implementation of bitscan idiom like "cttz_bitscan_shl" and "cttz_bitscan_shl_phi" (in cttz.ll) have the preheader that checks the "first bit" of the input (check it is zero). Here is to generalize the native pattern 1: preheader is generalized to any block that dominates the loop; 2. "initial bit" is generalized to "first <x> bits where <x> is within the range of bit indexes that checked by the loop"

I don't have a complex real world example, but thought this help recognize cttz pattern out of more than just the naive implementation with a few more checks.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55877