Promote bswap i16, i32, and i64 to be allowed llvm intrinsics.

lib/Analysis/NaCl/PNaClABIVerifyModule.cpp

 //===- PNaClABIVerifyModule.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 module-level PNaCl ABI requirements (specifically those that do not
@@ skipping 104 matching lines @@
       Reporter->addError() << GVTypeName << GV->getName()
                            << " has disallowed linkage type: "
                            << linkageName(GV->getLinkage()) << "\n";
   }
   if (GV->hasSection()) {
     Reporter->addError() << GVTypeName << GV->getName() <<
         " has disallowed \"section\" attribute\n";
   }
 }
 
+static bool IntTypeAcceptable(unsigned width,
+                              const ArrayRef<unsigned>& AcceptableSizes) {
+  for (unsigned i = 0; i < AcceptableSizes.size(); ++i)

[eliben, 2013/05/15 23:34:54]

Would it be cleaner to iterate over it (it has iterator support)?

[jvoung (off chromium), 2013/05/16 16:45:00]

Done.

+    if (AcceptableSizes[i] == width)
+      return true;
+  return false;
+}
+
+static bool FunctionHasIntArg1(const Function* F,

[eliben, 2013/05/15 23:34:54]

The name is not entirely clear... also you may be over-generalizing here just
for one case ;-)

[jvoung (off chromium), 2013/05/16 16:45:00]

Well, I assume I'll be checking parameter types for some of the other overloaded
intrinsics.

For now, just keeping the TypeAcceptable() function, and replacing this one with
isWhitelistedBswap().

+                               const ArrayRef<unsigned>& AcceptableSizes) {
+  FunctionType* FT = F->getFunctionType();
+  if (FT->getNumParams() != 1)
+    return false;
+  return IntTypeAcceptable(FT->getParamType(0)->getIntegerBitWidth(),
+                           AcceptableSizes);
+}
+
 bool PNaClABIVerifyModule::IsWhitelistedIntrinsic(const Function* F,
                                                   unsigned ID) {
   // Keep 3 categories of intrinsics for now.
   // (1) Allowed always
   // (2) Never allowed
   // (3) "Dev" intrinsics, which may or may not be allowed.
   // "Dev" intrinsics are controlled by the PNaClABIAllowDevIntrinsics flag.
   // Please keep these sorted or grouped in a sensible way, within
   // each category.
   switch(ID) {
     // Disallow by default.
     default: return false;
     // (1) Always allowed.
+    case Intrinsic::bswap: {

[eliben, 2013/05/15 23:34:54]

I think this would be clearer in a method - isWhitelistedBswap. Also mention
that it whitelists certain overloads in a comment. The method can then have
accepted_sizes as static const.

[jvoung (off chromium), 2013/05/16 16:45:00]

Done.

+      unsigned accepted_sizes[] = { 16, 32, 64 };
+      return FunctionHasIntArg1(F, accepted_sizes);
+    }
     case Intrinsic::invariant_end:
     case Intrinsic::invariant_start:
     case Intrinsic::lifetime_end:
     case Intrinsic::lifetime_start:
     case Intrinsic::memcpy:
     case Intrinsic::memmove:
     case Intrinsic::memset:
     case Intrinsic::nacl_read_tp:
     case Intrinsic::nacl_setjmp:
     case Intrinsic::nacl_longjmp:
@@ skipping 24 matching lines @@
     // Var-args handling is done w/out intrinsics.
     case Intrinsic::vacopy:
     case Intrinsic::vaend:
     case Intrinsic::vastart:
       return false;
 
     // (3) Dev intrinsics.
     case Intrinsic::dbg_declare:
     case Intrinsic::dbg_value:
       return PNaClABIAllowDevIntrinsics || PNaClABIAllowDebugMetadata;
-    case Intrinsic::bswap: // Support via compiler_rt if arch doesn't have it?
     case Intrinsic::cos: // Rounding not defined: support with fast-math?
     case Intrinsic::ctlz: // Support via compiler_rt if arch doesn't have it?
     case Intrinsic::ctpop: // Support via compiler_rt if arch doesn't have it?
     case Intrinsic::cttz: // Support via compiler_rt if arch doesn't have it?
     case Intrinsic::exp: // Rounding not defined: support with fast-math?
     case Intrinsic::exp2: // Rounding not defined: support with fast-math?
     case Intrinsic::expect: // From __builtin_expect.
-    case Intrinsic::flt_rounds:
+    case Intrinsic::flt_rounds: // For FLT_ROUNDS macro from float.h.
+      // We do not have fesetround() in newlib, can we return a
+      // consistent rounding mode though?
     case Intrinsic::log: // Rounding not defined: support with fast-math?
     case Intrinsic::log2: // Rounding not defined: support with fast-math?
     case Intrinsic::log10: // Rounding not defined: support with fast-math?
     case Intrinsic::nacl_target_arch: // Used by translator self-build.
     case Intrinsic::pow: // Rounding is supposed to be the same as libm.
     case Intrinsic::powi: // Rounding not defined: support with fast-math?
     case Intrinsic::prefetch: // Could ignore if target doesn't support?
     case Intrinsic::sin: // Rounding not defined: support with fast-math?
-    case Intrinsic::sqrt:
+    case Intrinsic::sqrt: // Rounding is defined, but setting errno up to libm.
     case Intrinsic::stackrestore: // Used to support C99 VLAs.
-    case Intrinsic::stacksave:
+    case Intrinsic::stacksave: // Used to support C99 VLAs.
     // the *_with_overflow return struct types, so we'll need to fix these.
     case Intrinsic::sadd_with_overflow: // Introduced by -ftrapv
     case Intrinsic::ssub_with_overflow:
     case Intrinsic::uadd_with_overflow:
     case Intrinsic::usub_with_overflow:
     case Intrinsic::smul_with_overflow:
     case Intrinsic::umul_with_overflow: // Introduced by c++ new[x * y].
       return PNaClABIAllowDevIntrinsics;
   }
 }
@@ skipping 107 matching lines @@
 }
 
 char PNaClABIVerifyModule::ID = 0;
 INITIALIZE_PASS(PNaClABIVerifyModule, "verify-pnaclabi-module",
                 "Verify module for PNaCl", false, true)
 
 ModulePass *llvm::createPNaClABIVerifyModulePass(
     PNaClABIErrorReporter *Reporter) {
   return new PNaClABIVerifyModule(Reporter);
 }