PNaCl: Add support for GCC/LLVM vector extensions

lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp

Index: lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp
diff --git a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp
index c1c909cc46618e6880b0df260b08ecdf8d173fa6..f98d2769e19e83ac4b612279844a368f9f69f26a 100644
--- a/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp
+++ b/lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp
@@ -88,6 +88,7 @@ bool PNaClABIVerifyFunctions::IsWhitelistedMetadata(unsigned MDKind) {
 
 // A valid pointer type is either:
 //  * a pointer to a valid PNaCl scalar type (except i1), or
+//  * a pointer to a valid PNaCl vector type (except i1 vector), or

[Mark Seaborn, 2014/04/09 15:03:03]

If you're not allowing loads/stores of vectors yet, you shouldn't need to allow
pointers to vectors.  If you leave out the change to isValidPointerType, then
you don't need to change load/store below.

[JF, 2014/04/15 18:22:15]

Done, and I updated the corresponding tests.

 //  * a function pointer (with valid argument and return types).
 //
 // i1 is disallowed so that all loads and stores are a whole number of
@@ -98,8 +99,10 @@ static bool isValidPointerType(Type *Ty) {
     if (PtrTy->getAddressSpace() != 0)
       return false;
     Type *EltTy = PtrTy->getElementType();
-    if (PNaClABITypeChecker::isValidScalarType(EltTy) &&
-        !EltTy->isIntegerTy(1))
+    if ((PNaClABITypeChecker::isValidScalarType(EltTy) &&
+         !EltTy->isIntegerTy(1)) ||
+        (PNaClABITypeChecker::isValidVectorType(EltTy) &&
+         !cast<VectorType>(EltTy)->getElementType()->isIntegerTy(1)))
       return true;
     if (FunctionType *FTy = dyn_cast<FunctionType>(EltTy))
       return PNaClABITypeChecker::isValidFunctionType(FTy);
@@ -152,6 +155,16 @@ static bool isValidScalarOperand(const Value *Val) {
           isa<UndefValue>(Val));
 }
 
+static bool isValidVectorOperand(const Value *Val) {
+  // The types of Instructions and Arguments are checked elsewhere.
+  if (isa<Instruction>(Val) || isa<Argument>(Val))
+    return true;
+  // Contrary to scalars, constant vector values aren't allowed on
+  // instructions, except for aggregate zero and undefined.
+  return PNaClABITypeChecker::isValidVectorType(Val->getType()) &&
+         (isa<ConstantAggregateZero>(Val) || isa<UndefValue>(Val));
+}
+
 static bool isAllowedAlignment(unsigned Alignment, Type *Ty) {
   // Non-atomic integer operations must always use "align 1", since we
   // do not want the backend to generate code with non-portable
@@ -163,6 +176,9 @@ static bool isAllowedAlignment(unsigned Alignment, Type *Ty) {
   // To reduce the set of alignment values that need to be encoded in
   // pexes, we disallow other alignment values.  We require alignments
   // to be explicit by disallowing Alignment == 0.
+  //
+  // Vector memory accesses go through their scalar elements, there is
+  // therefore no such thing as vector alignment.
   return Alignment == 1 ||
          (Ty->isDoubleTy() && Alignment == 8) ||
          (Ty->isFloatTy() && Alignment == 4);
@@ -253,9 +269,7 @@ const char *PNaClABIVerifyFunctions::checkInstruction(const Instruction *Inst) {
     case Instruction::Resume:
     // indirectbr may interfere with streaming
     case Instruction::IndirectBr:
-    // No vector instructions yet
-    case Instruction::ExtractElement:
-    case Instruction::InsertElement:
+    // TODO(jfb) Figure out ShuffleVector.
     case Instruction::ShuffleVector:
     // ExtractValue and InsertValue operate on struct values.
     case Instruction::ExtractValue:
@@ -312,10 +326,31 @@ const char *PNaClABIVerifyFunctions::checkInstruction(const Instruction *Inst) {
     case Instruction::SRem:
     case Instruction::Shl:
     case Instruction::LShr:
-    case Instruction::AShr:
-      if (Inst->getOperand(0)->getType()->isIntegerTy(1))
+    case Instruction::AShr: {
+      Type *Op0Ty = Inst->getOperand(0)->getType();
+      VectorType *Op0VTy = dyn_cast<VectorType>(Op0Ty);
+      if (Op0Ty->isIntegerTy(1) ||
+          (Op0VTy && Op0VTy->getElementType()->isIntegerTy(1)))

[Jim Stichnoth, 2014/04/04 23:08:17]

Is it possible to reasonably restructure this in the more usual LLVM way:

  if (VectorType *Op0VTy = dyn_cast<VectorType>(Op0Ty)) { ... }

which avoids the separate null check on Op0VTy?  I would be tempted to revert to
the original code and then add:

  if (VectorType *Op0VTy = dyn_cast<VectorType>(Op0Ty)) {
    if (Op0VTy->getElementType()->isIntegerTy(1))
      return "arithmetic on i1";
  }

(The constant string is duplicated but overall it's easier to read.)

[JF, 2014/04/15 01:52:27]

Done, I separated the error message to distinguish scalar and vector, so in the
end the message is clearer.

         return "arithmetic on i1";
       break;
+    }
+
+    // Vector.
+    case Instruction::ExtractElement:
+    case Instruction::InsertElement: {
+      // Insert and extract element are restricted to constant indices
+      // that are in range to prevent undefined behavior.
+      Value *Vec = Inst->getOperand(0);
+      Value *Idx = Inst->getOperand(
+          Instruction::InsertElement == Inst->getOpcode() ? 2 : 1);
+      if (!isa<ConstantInt>(Idx))
+        return "non-constant vector insert/extract index";
+      const APInt &I = cast<ConstantInt>(Idx)->getValue();
+      unsigned NumElements = cast<VectorType>(Vec->getType())->getNumElements();
+      if (!I.ult(NumElements))
+        return "out of range vector insert/extract index";
+      break;
+    }
 
     // Memory accesses.
     case Instruction::Load: {
@@ -325,6 +360,8 @@ const char *PNaClABIVerifyFunctions::checkInstruction(const Instruction *Inst) {
         return "atomic load";
       if (Load->isVolatile())
         return "volatile load";
+      if (Load->getType()->isVectorTy())
+        return "vector load";
       if (!isAllowedAlignment(Load->getAlignment(),
                               Load->getType()))
         return "bad alignment";
@@ -339,6 +376,8 @@ const char *PNaClABIVerifyFunctions::checkInstruction(const Instruction *Inst) {
         return "atomic store";
       if (Store->isVolatile())
         return "volatile store";
+      if (Store->getValueOperand()->getType()->isVectorTy())
+        return "vector store";
       if (!isAllowedAlignment(Store->getAlignment(),
                               Store->getValueOperand()->getType()))
         return "bad alignment";
@@ -392,6 +431,7 @@ const char *PNaClABIVerifyFunctions::checkInstruction(const Instruction *Inst) {
              ArgNum < E; ++ArgNum) {
           const Value *Arg = Call->getArgOperand(ArgNum);
           if (!(isValidScalarOperand(Arg) ||
+                isValidVectorOperand(Arg) ||
                 isNormalizedPtr(Arg) ||
                 isa<MDNode>(Arg)))
             return "bad intrinsic operand";
@@ -476,10 +516,11 @@ const char *PNaClABIVerifyFunctions::checkInstruction(const Instruction *Inst) {
   }
 
   // Check the instruction's operands.  We have already checked any
-  // pointer operands.  Any remaining operands must be scalars.
+  // pointer operands.  Any remaining operands must be scalars or vectors.
   for (unsigned OpNum = 0, E = Inst->getNumOperands(); OpNum < E; ++OpNum) {
     if (OpNum != PtrOperandIndex &&
-        !isValidScalarOperand(Inst->getOperand(OpNum)))
+        !(isValidScalarOperand(Inst->getOperand(OpNum)) ||
+          isValidVectorOperand(Inst->getOperand(OpNum))))
       return "bad operand";
   }
 
@@ -516,14 +557,19 @@ bool PNaClABIVerifyFunctions::runOnFunction(Function &F) {
       // check the reason for rejection.
       const char *Error = checkInstruction(BBI);
       // Check the instruction's result type.
+      bool BadResult = false;
       if (!Error && !(PNaClABITypeChecker::isValidScalarType(Inst->getType()) ||
+                      PNaClABITypeChecker::isValidVectorType(Inst->getType()) ||
                       isNormalizedPtr(Inst) ||
                       isa<AllocaInst>(Inst))) {
         Error = "bad result type";
+        BadResult = true;
       }
       if (Error) {
-        Reporter->addError() << "Function " << F.getName() <<
-          " disallowed: " << Error << ": " << *BBI << "\n";
+        Reporter->addError()
+            << "Function " << F.getName() << " disallowed: " << Error << ": "
+            << (BadResult ? PNaClABITypeChecker::getTypeName(BBI->getType())
+                          : "") << " " << *BBI << "\n";
       }
 
       // Check instruction attachment metadata.