[PATCH] D140796: Improve ComputeNumSignBits to handle Trunc.

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:3329
+      Tmp = ComputeNumSignBits(U->getOperand(0), Depth + 1, Q);
+      unsigned OperandTyBits = Q.DL.getTypeSizeInBits(U->getOperand(0)->getType());
+      if (Tmp > (OperandTyBits - TyBits)) {
----------------
I don't think this is right for vectors. You're taking the size of the full vector here, not of the vector elements. Using `U->getOperand(0)->getType()->getScalarSizeInBits()` should be fine here (as trunc is only defined on integers, so we don't need fancy DL machinery).

Please also add a vector test.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:3330
+      unsigned OperandTyBits = Q.DL.getTypeSizeInBits(U->getOperand(0)->getType());
+      if (Tmp > (OperandTyBits - TyBits)) {
+        return Tmp - (OperandTyBits - TyBits);
----------------
nit: Omit braces for single-line if.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:3334
+
       break;
+    }
----------------
return 1 here, we don't want to fall through to computeKnownBits (the ComptueNumSignBits will have already used it if possible).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140796