[PATCH] D10496: Extending Vector GetElementPtr - please review

[hfinkel at anl.gov]

hfinkel added a comment.

Minor issues.


================
Comment at: docs/LangRef.rst:6232
@@ +6231,3 @@
+arguments should have the same number of elements, and every scalar argument
+will be effectively broadcasted into a vector during address calculation.
+
----------------
broadcasted -> broadcast

(broadcasted is the past tense, so 'was broadcasted' but 'will be broadcast')


================
Comment at: docs/LangRef.rst:6279
@@ +6278,3 @@
+    ; load 8 elements from array B into A
+    %A = call <8 x double> @llvm.masked.gather.v8f64(<8 x double*> %ptrs,
+         i32 8, <8 x i1> %mask, <8 x double> %passthru)
----------------
Add this back in your patch for masked gather

================
Comment at: include/llvm/IR/Instructions.h:847
@@ -844,1 +846,3 @@
+        PointeeType->getScalarType() == cast<PointerType>
+        (Ptr->getType()->getScalarType())->getElementType()->getScalarType());
     return new (Values) GetElementPtrInst(PointeeType, Ptr, IdxList, Values,
----------------
Missing indent.

================
Comment at: include/llvm/IR/Instructions.h:861
@@ -855,1 +860,3 @@
+      assert(PointeeType == cast<PointerType>
+      (Ptr->getType()->getScalarType())->getElementType()->getScalarType());
     else
----------------
Missing indent.

================
Comment at: lib/AsmParser/LLParser.cpp:5546
@@ +5545,3 @@
+  // All vector parameters should have the same vector width.
+  unsigned GepWidth = BaseType->isVectorTy() ?
+    cast<VectorType>(BaseType)->getNumElements() : 0;
----------------
I'd name this GEPWidth.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2764
@@ +2763,3 @@
+  // Normalize Vector GEP - all scalar operands should be converted to the
+  // splat vector
+  unsigned VectorWidth =
----------------
Comments should end with a period.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:2766
@@ +2765,3 @@
+  unsigned VectorWidth =
+   I.getType()->isVectorTy()? cast<VectorType>(I.getType())->getVectorNumElements() : 0;
+
----------------
Line is too long.

================
Comment at: lib/IR/Verifier.cpp:2554
@@ -2557,4 +2553,3 @@
     // Additional checks for vector GEPs.
-    unsigned GepWidth = GEP.getPointerOperandType()->getVectorNumElements();
-    Assert(GepWidth == GEP.getType()->getVectorNumElements(),
-           "Vector GEP result width doesn't match operand's", &GEP);
+    unsigned GepWidth = cast<VectorType>(GEP.getType())->getVectorNumElements();
+    if (GEP.getPointerOperandType()->isVectorTy())
----------------
GEPWidth


rL LLVM

http://reviews.llvm.org/D10496