[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) {
----------------
Why are we doing this a different way than detectShiftUntilZeroIdiom? And why does the base need to be constant?


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1578
+  for (Use &U : VarX->uses()) {
+    auto *UI = dyn_cast<Instruction>(U.getUser());
+    if (!UI)
----------------
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?


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1761
+  bool FoundBitScanIdiom = false;
   if (!detectShiftUntilZeroIdiom(CurLoop, *DL, IntrinID, InitX,
+                                 CntInst, CntPhi, DefX)) {
----------------
Can you write this as

```
bool FoundBitScanIdiom;
if (detectShiftUntilZero...)
  FoundBitScanIdiom = false;
else if (detectShiftUntilZero...)
  FoundBitCanIdiom = true;
else
  return false
```

That should make it easier to add a third idiom in the future.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55877