[PATCH] D20931: [X86] Reduce the width of multiplification when its operands are extended from i8 or i16

[Michael Kuperstein via llvm-commits]

mkuper accepted this revision.
mkuper added a reviewer: mkuper.
mkuper added a comment.
This revision is now accepted and ready to land.

LGTM too.
I have a couple of comments, but they're rather half-baked, feel free to ignore them if they don't make sense to you.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:26446
@@ +26445,3 @@
+  bool IsPositive[2] = {false, false};
+  for (unsigned i = 0, e = N->getNumOperands(); i < e; i++) {
+    SDValue Opd = N->getOperand(i);
----------------
Nit - it's a bit misleading to have N->getNumOperands() here, and then index into an array of size 2.
Maybe assert N->getNumOperands() == 2 (or just use 2 here, but that's less self-documenting, I guess).

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:26470
@@ +26469,3 @@
+          continue;
+        auto *CN = dyn_cast<ConstantSDNode>(SubOp);
+        if (!CN)
----------------
Perhaps use ComputeNumSignBits on the BUILD_VECTOR elements as well, instead of checking for const/undef?
Although I'm really not sure if that gains anything in practice, and it won't make the code significantly smaller, so it's may not be worth the overhead.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:26480
@@ +26479,3 @@
+      SignBits[i] = DAG.ComputeNumSignBits(Opd);
+      if (Opd.getOpcode() == ISD::ZERO_EXTEND)
+        IsPositive[i] = true;
----------------
Maybe computeKnownBits instead of special-casing ZERO_EXTEND?
Although, the same as above applies, not at all sure it's worth the overhead.


Repository:
  rL LLVM

http://reviews.llvm.org/D20931