[PATCH] D45721: [X86] Lowering PACK*S (pack with saturation) intrinsics to native IR (LLVM side)

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/lib/IR/AutoUpgrade.cpp:1054
+    else
+      ShuffleMask.push_back(i1++);
+  Value *Res = Builder.CreateShuffleVector(A, B, ShuffleMask);
----------------
This shuffle mask creation is very confusing, please can you make it more obvious (see createPackShuffleMask in X86ISelLowering.cpp)


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:34777
+      return DAG.getNode(Opcode, DL, VT, A, B);
+  }
   if (TLI.isTypeLegal(InVT) && TLI.isTypeLegal(VT) &&
----------------
This seems all rather messy - please can you try to clean it up? 


================
Comment at: llvm/test/CodeGen/X86/sse41-intrinsics-x86.ll:161
-  ret <8 x i16> %res
-}
-
----------------
Please can you leave these tests for now - add a FIXME comment about this PACKS being upgraded if you wish.


================
Comment at: llvm/test/CodeGen/X86/vector-shuffle-combining-avx2.ll:879
-}
-declare <32 x i8> @llvm.x86.avx2.packuswb(<16 x i16>, <16 x i16>) nounwind readnone
-
----------------
Please can you leave these tests for now - add a FIXME comment about this PACKS being upgraded if you wish.


https://reviews.llvm.org/D45721