[PATCH] D38312: adding pattern for broadcastm

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:6605
 
+// check if the current node of build vector is a zero extended vedtor.
+// if so, return the value extended.
----------------
vedtor->vector

Also the first word of each sentence in the comment should be capitalized.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:6628
+    } else if (!(isNullConstant(Op->getOperand(i)) ||
+               Op->getOperand(i).isUndef()))
+      return SDValue();
----------------
I think this line should be indented 2 additional spaces?


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:6635
+  EltType = MVT::getIntegerVT(ExtVTSize);
+  NumElt = Op->getNumOperands() / Delta;
+  return ExtValue;
----------------
Am I missing something or is this variable and several above it dead?


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:6667
+  // create (VBROADCASTM v2i1 X)
+  if (Subtarget.hasCDI() && (VT.getSizeInBits() == 512 || Subtarget.hasVLX())) {
+    MVT EltType;
----------------
Use is512BitVector here.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:6674
+      if (BOperand.getSimpleValueType().getVectorElementType() == MVT::i1) {
+        if ((EltType == MVT::i64 && VT.getVectorElementType() == MVT::i8) ||  //for broadcastmb2q
+            (EltType == MVT::i32 && VT.getVectorElementType() == MVT::i16)) { //for broadcastmw2d
----------------
Does this overflow 80 columns?


https://reviews.llvm.org/D38312