[PATCH] D14588: [X86][SSE] Transform truncation from v8i32/v16i32 to v8i8/v16i8 into bitand and X86ISD::PACKUS operations during DAG combine.

[Simon Pilgrim via llvm-commits]

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

LGTM with a couple of (optional) minor corrections.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:26141
@@ +26140,3 @@
+  EVT InVT = Regs[0].getValueType();
+  EVT WideElemVT = InVT.getVectorElementType();
+  SDLoc DL(N);
----------------
Maybe use the same naming convention for the out / in VTs?

OutVT + OutSVT
InVT + InSVT

Makes it easier to track.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:26146
@@ +26145,3 @@
+  SDValue MaskVal =
+      DAG.getConstant(ElemVT == MVT::i8 ? 255 : 65535, DL, WideElemVT);
+  SDValue MaskVec = DAG.getNode(
----------------
Can you please assert that OutSVT is a i8 / i16 before this? Maybe use APInt to create the mask?

================
Comment at: test/CodeGen/X86/vector-trunc.ll:156
@@ +155,3 @@
+
+define void @trunc8i64_8i8(<8 x i64> %a) {
+; SSE2-LABEL: trunc8i64_8i8:
----------------
Its a pity that the SSSE3/SSE41 codegen is still so poor - can you add a FIXME explaining what still needs doing?


http://reviews.llvm.org/D14588