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

Wei Mi via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 8 13:33:01 PDT 2016


wmi added inline comments.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:673
@@ -672,2 +672,3 @@
   case ISD::MUL:
+  case ISD::MULHU:
   case ISD::FADD:
----------------
eli.friedman wrote:
> Do you need MULHS here too?  (Maybe missing a test for this?)
Ah sorry. Fixed. Add a test mul_16xi16_sext for it. 

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:26453
@@ +26452,3 @@
+      // If Opd is anyext(i8), we suppose the value range of Opd is 0 to 127,
+      // so we don't care whether anyext is either sext or zext.
+      if (Opd.getOperand(0).getValueType().getVectorElementType() == MVT::i8)
----------------
eli.friedman wrote:
> This comment is confusing; the operand isn't actually guaranteed to be between 0 and 127.  (The transform is safe because we can assume an appropriate number of leading sign/zero bits.)
Fixed. 

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:26478
@@ +26477,3 @@
+        SignBits[i] = std::min(SignBits[i], DAG.ComputeNumSignBits(SubOp));
+      }
+    } else {
----------------
eli.friedman wrote:
> Probably better to use APInt::getNumSignBits here.
Fixed.


Repository:
  rL LLVM

http://reviews.llvm.org/D20931





More information about the llvm-commits mailing list