[PATCH] D54876: [DemandedBits] Add support for funnel shifts

[Nikita Popov via Phabricator via llvm-commits]

nikic marked 4 inline comments as done.
nikic added inline comments.


================
Comment at: lib/Analysis/DemandedBits.cpp:147
+      case Intrinsic::fshr:
+        if (auto *SA = dyn_cast<ConstantInt>(II->getOperand(2))) {
+          // Normalize to funnel shift left. APInt shifts of BitWidth are well-
----------------
spatel wrote:
> Is this analysis completely ignoring vector integer instructions? 
> 
> It would still be good to use 'match(X, m_APInt())' in case we ever upgrade the whole thing to handle vectors. But if the answer to the above question is 'yes', then a TODO comment is ok for now.
Yes, DemandedBits currently only operates on scalar integer instructions. I originally wanted to use match() here, but seeing how all the surrounding code uses ConstantInt casts, I went with that.

I might look into adding vector support for DemandedBits, it does not seem too complicated.


================
Comment at: lib/Analysis/DemandedBits.cpp:154-156
+          if (OperandNo == 0)
+            AB = AOut.lshr(ShiftAmt);
+          else if (OperandNo == 1)
----------------
spatel wrote:
> Can we do something like D54478 for operand 2 in a follow-up patch?
I went ahead and included it here, as it seems like a small enough change :)


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

https://reviews.llvm.org/D54876