[PATCH] [x86] Teach the target-specific combining how to aggressively foldhalf-shuffles, even looking through intervening instructions in a chain.

[Jim Grosbach]

Nitty comments inline. Code looks great.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:19002
@@ +19001,3 @@
+      // We can only handle pshufd if the half we are combining either stay in
+      // their half, or switch halves. Bail if one of these isn't true.
+      SmallVector<int, 4> VMask = getPSHUFShuffleMask(V);
----------------
grammar. "if the halves" maybe what you meant?

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:19006
@@ +19005,3 @@
+      // Bail if the PSHUFD doesn't keep both dwords being shuffled in a single
+      // half of its input.
+      if (!((VMask[DOffset + 0] < 2 && VMask[DOffset + 1] < 2) ||
----------------
"of it's input" sounds odd, but I'm not sure it's wrong. Can you rephrase?

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:19032
@@ +19031,3 @@
+
+  // Merge this nodes mask and our incoming mask (adjusted to account for all
+  // the pshufd instructions encountered).
----------------
s/nodes/node's/

================
Comment at: test/CodeGen/X86/vector-shuffle-combining.ll:11
@@ +10,3 @@
+; CHECK-SSE2-LABEL: @combine_pshuflw1
+; CHECK-SSE2:       # BB#0:
+; CHECK-SSE2-NEXT:    retq
----------------
I prefer not to look for these sorts of comments in test lines if at all possible and to just run llc with -asm-verbose=false. That enables explicit CHECK-NEXT for instructions right after the function label to make sure there's not extraneous stuff showing up, which looks like what you're trying to do here.

http://reviews.llvm.org/D4291