[PATCH] D36679: [InstCombine] Added support for: trunc(ashr(mul(sext(...), sext(...))) -> ashr(mul(...))

[Sanjay Patel via Phabricator via llvm-commits]

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

Thanks for breaking this up - LGTM.
See inline for a couple of nits.



================
Comment at: lib/Analysis/ValueTracking.cpp:2200
+  case Instruction::Mul: {
+    // The output of the Mul can at most twice the valid bits in the inputs.
+    Tmp = ComputeNumSignBits(U->getOperand(0), Depth + 1, Q);
----------------
can at -> can be at


================
Comment at: lib/Analysis/ValueTracking.cpp:2201-2203
+    Tmp = ComputeNumSignBits(U->getOperand(0), Depth + 1, Q);
+    if (Tmp == 1) return 1;  // Early out.
+    Tmp2 = ComputeNumSignBits(U->getOperand(1), Depth + 1, Q);
----------------
It's poor to have variables named 'Tmp'. These can be locals now, so let's give these meaningful names - SignBitsOp0/SignBitsOp1 or something like that.


https://reviews.llvm.org/D36679