PNaCl: Add support for GCC/LLVM vector extensions

lib/Analysis/NaCl/PNaClABIVerifyFunctions.cpp

 //===- PNaClABIVerifyFunctions.cpp - Verify PNaCl ABI rules ---------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // Verify function-level PNaCl ABI requirements.
@@ skipping 70 matching lines @@
   }
   return N.str();
 }
 
 bool PNaClABIVerifyFunctions::IsWhitelistedMetadata(unsigned MDKind) {
   return MDKind == LLVMContext::MD_dbg && PNaClABIAllowDebugMetadata;
 }
 
 // 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
 // bytes, and so that we do not need to define whether a store of i1
 // zero-extends.
 static bool isValidPointerType(Type *Ty) {
   if (PointerType *PtrTy = dyn_cast<PointerType>(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);
   }
   return false;
 }
 
 static bool isIntrinsicFunc(const Value *Val) {
   if (const Function *F = dyn_cast<Function>(Val))
     return F->isIntrinsic();
@@ skipping 32 matching lines @@
   if (isa<Instruction>(Val) || isa<Argument>(Val) || isa<BasicBlock>(Val))
     return true;
 
   // Allow some Constants.  Note that this excludes ConstantExprs.
   return PNaClABITypeChecker::isValidScalarType(Val->getType()) &&
          (isa<ConstantInt>(Val) ||
           isa<ConstantFP>(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
   // undefined behaviour (such as misaligned access faults) if user
   // code specifies "align 4" but uses a misaligned pointer.  As a
   // concession to performance, we allow larger alignment values for
   // floating point types.
   //
   // 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);
 }
 
 static bool hasAllowedAtomicRMWOperation(
     const NaCl::AtomicIntrinsics::AtomicIntrinsic *I, const CallInst *Call) {
   for (size_t P = 0; P != I->NumParams; ++P) {
     if (I->ParamType[P] != NaCl::AtomicIntrinsics::RMW)
       continue;
@@ skipping 70 matching lines @@
     // We expand GetElementPtr out into arithmetic.
     case Instruction::GetElementPtr:
     // VAArg is expanded out by ExpandVarArgs.
     case Instruction::VAArg:
     // Zero-cost C++ exception handling is not supported yet.
     case Instruction::Invoke:
     case Instruction::LandingPad:
     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:
     case Instruction::InsertValue:
     // Atomics should become NaCl intrinsics.
     case Instruction::AtomicCmpXchg:
     case Instruction::AtomicRMW:
     case Instruction::Fence:
       return "bad instruction opcode";
     default:
@@ skipping 36 matching lines @@
     // Binary operations
     case Instruction::Add:
     case Instruction::Sub:
     case Instruction::Mul:
     case Instruction::UDiv:
     case Instruction::SDiv:
     case Instruction::URem:
     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: {
       const LoadInst *Load = cast<LoadInst>(Inst);
       PtrOperandIndex = Load->getPointerOperandIndex();
       if (Load->isAtomic())
         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";
       if (!isNormalizedPtr(Inst->getOperand(PtrOperandIndex)))
         return "bad pointer";
       break;
     }
     case Instruction::Store: {
       const StoreInst *Store = cast<StoreInst>(Inst);
       PtrOperandIndex = Store->getPointerOperandIndex();
       if (Store->isAtomic())
         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";
       if (!isNormalizedPtr(Inst->getOperand(PtrOperandIndex)))
         return "bad pointer";
       break;
     }
 
     // Casts.
     case Instruction::BitCast:
@@ skipping 33 matching lines @@
       if (Call->getCallingConv() != CallingConv::C)
         return "bad calling convention";
 
       // Intrinsic calls can have multiple pointer arguments and
       // metadata arguments, so handle them specially.
       if (const IntrinsicInst *Call = dyn_cast<IntrinsicInst>(Inst)) {
         for (unsigned ArgNum = 0, E = Call->getNumArgOperands();
              ArgNum < E; ++ArgNum) {
           const Value *Arg = Call->getArgOperand(ArgNum);
           if (!(isValidScalarOperand(Arg) ||
+                isValidVectorOperand(Arg) ||
                 isNormalizedPtr(Arg) ||
                 isa<MDNode>(Arg)))
             return "bad intrinsic operand";
         }
 
         // Disallow alignments other than 1 on memcpy() etc., for the
         // same reason that we disallow them on integer loads and
         // stores.
         if (const MemIntrinsic *MemOp = dyn_cast<MemIntrinsic>(Call)) {
           // Avoid the getAlignment() method here because it aborts if
@@ skipping 64 matching lines @@
         if (!isValidScalarOperand(Case.getCaseValue()))
           return "bad switch case";
       }
 
       // Allow the instruction and skip the later checks.
       return NULL;
     }
   }
 
   // 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";
   }
 
   // Check arithmetic attributes.
   if (const OverflowingBinaryOperator *Op =
           dyn_cast<OverflowingBinaryOperator>(Inst)) {
     if (Op->hasNoUnsignedWrap())
       return "has \"nuw\" attribute";
     if (Op->hasNoSignedWrap())
       return "has \"nsw\" attribute";
@@ skipping 16 matching lines @@
            FI != FE; ++FI) {
     for (BasicBlock::const_iterator BBI = FI->begin(), BBE = FI->end();
              BBI != BBE; ++BBI) {
       const Instruction *Inst = BBI;
       // Check the instruction opcode first.  This simplifies testing,
       // because some instruction opcodes must be rejected out of hand
       // (regardless of the instruction's result type) and the tests
       // 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.
       SmallVector<std::pair<unsigned, MDNode*>, 4> MDForInst;
       BBI->getAllMetadata(MDForInst);
 
       for (unsigned i = 0, e = MDForInst.size(); i != e; i++) {
         if (!IsWhitelistedMetadata(MDForInst[i].first)) {
           Reporter->addError()
               << "Function " << F.getName()
@@ skipping 18 matching lines @@
 }
 
 char PNaClABIVerifyFunctions::ID = 0;
 INITIALIZE_PASS(PNaClABIVerifyFunctions, "verify-pnaclabi-functions",
                 "Verify functions for PNaCl", false, true)
 
 FunctionPass *llvm::createPNaClABIVerifyFunctionsPass(
     PNaClABIErrorReporter *Reporter) {
   return new PNaClABIVerifyFunctions(Reporter);
 }