[PATCH] D35700: DAGCombiner: Extend reduceBuildVecToTrunc to handle non-zero offset

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

A few minor comments, but looks almost ready.



================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14480
+  }
+  // Construct the truncate
   LLVMContext &Ctx = *DAG.getContext();
----------------
Construct the truncate.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:35833
+
+  assert(SrcVT.getVectorNumElements() == ShuffleMask.size() &&
+         "Element count mismatch");
----------------
assert for isShuffleMaskLegal to double check?


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:35844
+    if (ShuffleMask[i*Stride] != Offset + i * Stride)
+      return false;
+  // For 32-bit elements VPERMD is better than shuffle+truncate.
----------------
You could probably replace all of this with:
```
if(!is128BitLaneCrossingShuffleMask(SrcVT, ShuffleMask))
  return false;
```


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:35847
+  // TODO: After we improve lowerBuildVector, add execption for VPERMW.
+  return (SrcVT.getScalarSizeInBits() != 32 && Subtarget.hasAVX2());
+}
----------------
Better to do this at the top to early-out if we don't support it. Then just return true at the end.


https://reviews.llvm.org/D35700