[PATCH] D16072: [X86][SSE] Add INSERTPS target shuffle combines.

[Sanjay Patel via llvm-commits]

spatel added a comment.

I applied the patch to r258047, and I get a 'make check' failure on:
FAIL: LLVM :: CodeGen/X86/merge-consecutive-loads-128.ll (6972 of 15632)


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:6938
@@ +6937,3 @@
+                                                      SDValue V1, SDValue V2) {
+  SmallBitVector KnownZero(Mask.size(), false);
+
----------------
Why do we need this intermediate variable? Ie, couldn't we just set the appropriate elements of the mask rather than this bitvector which then gets copied to the mask?

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:23605
@@ +23604,3 @@
+/// not just zeroable.
+static bool ResolveTargetShuffleZeroElts(SDValue N,
+                                         SmallVectorImpl<int> &Mask) {
----------------
"resolveTarget..."

or if the previous inline comment applies, combine the 2 helper functions and call it "setShuffleMaskZeroElements" ?

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:23756
@@ +23755,3 @@
+      int M = TargetMask[i];
+      if ((InsertPSMask & (1u << i)) || (i == DstIdx)) {
+        // No change if element is already zero or the inserted element.
----------------
warning: comparison of integers of different signs


Repository:
  rL LLVM

http://reviews.llvm.org/D16072