[PATCH] Fix a broadcast related regression on the vector shuffle lowering.

[Chandler Carruth]

Minor comments on the code.

Please fold these tests into the existing 256-bit vector shuffle tests and use the same style of file check assertions.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:7900
@@ +7899,3 @@
+  // we can combine with the broadcast.
+  while (true) {
+    if (V.getOpcode() == ISD::CONCAT_VECTORS) {
----------------
for (;;) { is my preferred spelling of an loop without a condition.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:7901
@@ +7900,3 @@
+  while (true) {
+    if (V.getOpcode() == ISD::CONCAT_VECTORS) {
+      int OperandSize = Mask.size() / V.getNumOperands();
----------------
Switch on the opcode rather than an if chain. It makes it much more clear what is going on if we extend this later. Using continue makes it easy to handle the "double break" issue.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:7908
@@ +7907,3 @@
+      SDValue SDIdx = V.getOperand(2);
+      if (!isa<ConstantSDNode>(SDIdx))
+        break;
----------------
dyn_cast, and break if null. that way we only go through the cast machinery once.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:7913
@@ +7912,3 @@
+          BroadcastIdx <
+              Idx + (int)VInner.getValueType().getVectorNumElements()) {
+        BroadcastIdx -= Idx;
----------------
I would probably precompute some part of the upper bound into a variable to make the bounds test a bit easier to read.

http://reviews.llvm.org/D5745