[PATCH] D38312: adding pattern for broadcastm

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon reopened this revision.
RKSimon added a comment.

A few minor comments



================
Comment at: lib/Target/X86/X86ISelLowering.cpp:6608
+// If so, return the value extended.
+static SDValue isSplatZeroExtended(const BuildVectorSDNode *Op,
+                                   unsigned &NumElt, MVT &EltType) {
----------------
Describe the function NumElt and EltType, also I'm not convinced that 'isSplatZeroExtended' is suitably descriptive of what the function is actually doing.


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:6611
+  SDValue ExtValue = Op->getOperand(0);
+  unsigned Delta = Op->getNumOperands();
+
----------------
```
unsigned NumElts = Op->getNumOperands();
unsigned Delta = NumElts;
```


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:6613
+
+  for (unsigned i = 1; i < Op->getNumOperands(); i++) {
+    if (Op->getOperand(i) == ExtValue) {
----------------
Comment describing the search you're doing in the loop

```
for (unsigned i = 1; i < NumElts; i++) {
```


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:6624
+
+  for (unsigned i = Delta; i < Op->getNumOperands(); i++) {
+    if (i % Delta == 0) {
----------------
Comment the loop

```
for (unsigned i = Delta; i < NumElts; i++) {
```


================
Comment at: lib/Target/X86/X86ISelLowering.cpp:6633
+  unsigned EltSize =
+      Op->getSimpleValueType(0).getVectorElementType().getSizeInBits();
+  unsigned ExtVTSize = EltSize * Delta;
----------------
Op->getSimpleValueType(0).getScalaraSizeInBits()


https://reviews.llvm.org/D38312