From 45de76ea91e590f63496f49468fa51dcf4b2ba2a Mon Sep 17 00:00:00 2001 From: Tim Northover Date: Wed, 19 Feb 2014 10:37:05 +0000 Subject: [PATCH] ARM & AArch64: merge the semantic checking of NEON intrinsics There are two kinds of automatically generated tests for NEON intrinsics, both of which can be merged without adversely affecting users. 1. We check that a valid kind of __builtin_neon_XYZ overload is requested (e.g. we're not asking for a float32x4_t version when it only accepts integers. Since the __builtin_neon_XYZ intrinsics should only be used in arm_neon.h, relaxing this test and permitting AArch64 types for AArch32 should not cause a problem. The extra arm_neon.h definitions should be #ifdefed out anyway. 2. We check that intrinsics which take immediates are actually given compile-time constants within range. Since all NEON intrinsics should be backwards compatible, these tests should be identical on AArch64 and AArch32 anyway. This patch, therefore, merges the separate AArch64 and 32-bit checks. rdar://problem/16035743 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@201659 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Sema/Sema.h | 1 + lib/Sema/SemaChecking.cpp | 77 +++++------------- utils/TableGen/NeonEmitter.cpp | 143 ++++++++++++--------------------- 3 files changed, 72 insertions(+), 149 deletions(-) diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index 9cb6635c3f..ff21c1d284 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -7887,6 +7887,7 @@ private: ExprResult CheckBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall); + bool CheckNeonBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall); bool CheckARMBuiltinExclusiveCall(unsigned BuiltinID, CallExpr *TheCall); bool CheckARMBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall); bool CheckAArch64BuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall); diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index dfacba84ef..cfd042f945 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -388,24 +388,21 @@ static QualType getNeonEltType(NeonTypeFlags Flags, ASTContext &Context, llvm_unreachable("Invalid NeonTypeFlag!"); } -bool Sema::CheckAArch64BuiltinFunctionCall(unsigned BuiltinID, - CallExpr *TheCall) { - +bool Sema::CheckNeonBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) { llvm::APSInt Result; - uint64_t mask = 0; unsigned TV = 0; int PtrArgNum = -1; bool HasConstPtr = false; switch (BuiltinID) { -#define GET_NEON_AARCH64_OVERLOAD_CHECK +#define GET_NEON_OVERLOAD_CHECK #include "clang/Basic/arm_neon.inc" -#undef GET_NEON_AARCH64_OVERLOAD_CHECK +#undef GET_NEON_OVERLOAD_CHECK } // For NEON intrinsics which are overloaded on vector element type, validate // the immediate which specifies which variant to emit. - unsigned ImmArg = TheCall->getNumArgs() - 1; + unsigned ImmArg = TheCall->getNumArgs()-1; if (mask) { if (SemaBuiltinConstantArg(TheCall, ImmArg, Result)) return true; @@ -413,7 +410,7 @@ bool Sema::CheckAArch64BuiltinFunctionCall(unsigned BuiltinID, TV = Result.getLimitedValue(64); if ((TV > 63) || (mask & (1ULL << TV)) == 0) return Diag(TheCall->getLocStart(), diag::err_invalid_neon_type_code) - << TheCall->getArg(ImmArg)->getSourceRange(); + << TheCall->getArg(ImmArg)->getSourceRange(); } if (PtrArgNum >= 0) { @@ -423,7 +420,10 @@ bool Sema::CheckAArch64BuiltinFunctionCall(unsigned BuiltinID, Arg = ICE->getSubExpr(); ExprResult RHS = DefaultFunctionArrayLvalueConversion(Arg); QualType RHSTy = RHS.get()->getType(); - QualType EltTy = getNeonEltType(NeonTypeFlags(TV), Context, true); + + bool IsAArch64 = + Context.getTargetInfo().getTriple().getArch() == llvm::Triple::aarch64; + QualType EltTy = getNeonEltType(NeonTypeFlags(TV), Context, IsAArch64); if (HasConstPtr) EltTy = EltTy.withConst(); QualType LHSTy = Context.getPointerType(EltTy); @@ -442,9 +442,9 @@ bool Sema::CheckAArch64BuiltinFunctionCall(unsigned BuiltinID, switch (BuiltinID) { default: return false; -#define GET_NEON_AARCH64_IMMEDIATE_CHECK +#define GET_NEON_IMMEDIATE_CHECK #include "clang/Basic/arm_neon.inc" -#undef GET_NEON_AARCH64_IMMEDIATE_CHECK +#undef GET_NEON_IMMEDIATE_CHECK } ; @@ -466,6 +466,14 @@ bool Sema::CheckAArch64BuiltinFunctionCall(unsigned BuiltinID, return false; } +bool Sema::CheckAArch64BuiltinFunctionCall(unsigned BuiltinID, + CallExpr *TheCall) { + if (CheckNeonBuiltinFunctionCall(BuiltinID, TheCall)) + return true; + + return false; +} + bool Sema::CheckARMBuiltinExclusiveCall(unsigned BuiltinID, CallExpr *TheCall) { assert((BuiltinID == ARM::BI__builtin_arm_ldrex || BuiltinID == ARM::BI__builtin_arm_strex) && @@ -580,48 +588,8 @@ bool Sema::CheckARMBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) { return CheckARMBuiltinExclusiveCall(BuiltinID, TheCall); } - uint64_t mask = 0; - unsigned TV = 0; - int PtrArgNum = -1; - bool HasConstPtr = false; - switch (BuiltinID) { -#define GET_NEON_OVERLOAD_CHECK -#include "clang/Basic/arm_neon.inc" -#undef GET_NEON_OVERLOAD_CHECK - } - - // For NEON intrinsics which are overloaded on vector element type, validate - // the immediate which specifies which variant to emit. - unsigned ImmArg = TheCall->getNumArgs()-1; - if (mask) { - if (SemaBuiltinConstantArg(TheCall, ImmArg, Result)) - return true; - - TV = Result.getLimitedValue(64); - if ((TV > 63) || (mask & (1ULL << TV)) == 0) - return Diag(TheCall->getLocStart(), diag::err_invalid_neon_type_code) - << TheCall->getArg(ImmArg)->getSourceRange(); - } - - if (PtrArgNum >= 0) { - // Check that pointer arguments have the specified type. - Expr *Arg = TheCall->getArg(PtrArgNum); - if (ImplicitCastExpr *ICE = dyn_cast(Arg)) - Arg = ICE->getSubExpr(); - ExprResult RHS = DefaultFunctionArrayLvalueConversion(Arg); - QualType RHSTy = RHS.get()->getType(); - QualType EltTy = getNeonEltType(NeonTypeFlags(TV), Context, false); - if (HasConstPtr) - EltTy = EltTy.withConst(); - QualType LHSTy = Context.getPointerType(EltTy); - AssignConvertType ConvTy; - ConvTy = CheckSingleAssignmentConstraints(LHSTy, RHS); - if (RHS.isInvalid()) - return true; - if (DiagnoseAssignmentResult(ConvTy, Arg->getLocStart(), LHSTy, RHSTy, - RHS.get(), AA_Assigning)) - return true; - } + if (CheckNeonBuiltinFunctionCall(BuiltinID, TheCall)) + return true; // For NEON intrinsics which take an immediate value as part of the // instruction, range check them here. @@ -634,9 +602,6 @@ bool Sema::CheckARMBuiltinFunctionCall(unsigned BuiltinID, CallExpr *TheCall) { case ARM::BI__builtin_arm_vcvtr_d: i = 1; u = 1; break; case ARM::BI__builtin_arm_dmb: case ARM::BI__builtin_arm_dsb: l = 0; u = 15; break; -#define GET_NEON_IMMEDIATE_CHECK -#include "clang/Basic/arm_neon.inc" -#undef GET_NEON_IMMEDIATE_CHECK }; // We can't check the value of a dependent argument. diff --git a/utils/TableGen/NeonEmitter.cpp b/utils/TableGen/NeonEmitter.cpp index db954b2183..5b0084afdb 100644 --- a/utils/TableGen/NeonEmitter.cpp +++ b/utils/TableGen/NeonEmitter.cpp @@ -375,12 +375,8 @@ private: void emitIntrinsic(raw_ostream &OS, Record *R, StringMap &EmittedMap); void genBuiltinsDef(raw_ostream &OS); - void genOverloadTypeCheckCode(raw_ostream &OS, - StringMap &A64IntrinsicMap, - bool isA64TypeCheck); - void genIntrinsicRangeCheckCode(raw_ostream &OS, - StringMap &A64IntrinsicMap, - bool isA64RangeCheck); + void genOverloadTypeCheckCode(raw_ostream &OS); + void genIntrinsicRangeCheckCode(raw_ostream &OS); void genTargetTest(raw_ostream &OS, StringMap &EmittedMap, bool isA64TestGen); }; @@ -2911,17 +2907,12 @@ static unsigned RangeScalarShiftImm(const char mod, StringRef typestr) { /// Generate the ARM and AArch64 intrinsic range checking code for /// shift/lane immediates, checking for unique declarations. void -NeonEmitter::genIntrinsicRangeCheckCode(raw_ostream &OS, - StringMap &A64IntrinsicMap, - bool isA64RangeCheck) { +NeonEmitter::genIntrinsicRangeCheckCode(raw_ostream &OS) { std::vector RV = Records.getAllDerivedDefinitions("Inst"); StringMap EmittedMap; // Generate the intrinsic range checking code for shift/lane immediates. - if (isA64RangeCheck) - OS << "#ifdef GET_NEON_AARCH64_IMMEDIATE_CHECK\n"; - else - OS << "#ifdef GET_NEON_IMMEDIATE_CHECK\n"; + OS << "#ifdef GET_NEON_IMMEDIATE_CHECK\n"; for (unsigned i = 0, e = RV.size(); i != e; ++i) { Record *R = RV[i]; @@ -2956,19 +2947,6 @@ NeonEmitter::genIntrinsicRangeCheckCode(raw_ostream &OS, if (!ProtoHasScalar(Proto)) ck = ClassB; - // Do not include AArch64 range checks if not generating code for AArch64. - bool isA64 = R->getValueAsBit("isA64"); - if (!isA64RangeCheck && isA64) - continue; - - // Include ARM range checks in AArch64 but only if ARM intrinsics are not - // redefined by AArch64 to handle new types. - if (isA64RangeCheck && !isA64 && A64IntrinsicMap.count(Rename)) { - ClassKind &A64CK = A64IntrinsicMap[Rename]; - if (A64CK == ck && ck != ClassNone) - continue; - } - for (unsigned ti = 0, te = TypeVec.size(); ti != te; ++ti) { std::string namestr, shiftstr, rangestr; @@ -3070,16 +3048,22 @@ NeonEmitter::genIntrinsicRangeCheckCode(raw_ostream &OS, /// Generate the ARM and AArch64 overloaded type checking code for /// SemaChecking.cpp, checking for unique builtin declarations. void -NeonEmitter::genOverloadTypeCheckCode(raw_ostream &OS, - StringMap &A64IntrinsicMap, - bool isA64TypeCheck) { +NeonEmitter::genOverloadTypeCheckCode(raw_ostream &OS) { std::vector RV = Records.getAllDerivedDefinitions("Inst"); // Generate the overloaded type checking code for SemaChecking.cpp - if (isA64TypeCheck) - OS << "#ifdef GET_NEON_AARCH64_OVERLOAD_CHECK\n"; - else - OS << "#ifdef GET_NEON_OVERLOAD_CHECK\n"; + OS << "#ifdef GET_NEON_OVERLOAD_CHECK\n"; + + // We record each overload check line before emitting because subsequent Inst + // definitions may extend the number of permitted types (i.e. augment the + // Mask). Use std::map to avoid sorting the table by hash number. + struct OverloadInfo { + uint64_t Mask; + int PtrArgNum; + bool HasConstPtr; + }; + std::map OverloadMap; + typedef std::map::iterator OverloadIterator; for (unsigned i = 0, e = RV.size(); i != e; ++i) { Record *R = RV[i]; @@ -3108,21 +3092,6 @@ NeonEmitter::genOverloadTypeCheckCode(raw_ostream &OS, if (R->getSuperClasses().size() < 2) PrintFatalError(R->getLoc(), "Builtin has no class kind"); - // Do not include AArch64 type checks if not generating code for AArch64. - bool isA64 = R->getValueAsBit("isA64"); - if (!isA64TypeCheck && isA64) - continue; - - // Include ARM type check in AArch64 but only if ARM intrinsics - // are not redefined in AArch64 to handle new types, e.g. "vabd" is a SIntr - // redefined in AArch64 to handle an additional 2 x f64 type. - ClassKind ck = ClassMap[R->getSuperClasses()[1]]; - if (isA64TypeCheck && !isA64 && A64IntrinsicMap.count(Rename)) { - ClassKind &A64CK = A64IntrinsicMap[Rename]; - if (A64CK == ck && ck != ClassNone) - continue; - } - int si = -1, qi = -1; uint64_t mask = 0, qmask = 0; for (unsigned ti = 0, te = TypeVec.size(); ti != te; ++ti) { @@ -3170,26 +3139,41 @@ NeonEmitter::genOverloadTypeCheckCode(raw_ostream &OS, } if (mask) { - OS << "case NEON::BI__builtin_neon_"; - OS << MangleName(name, TypeVec[si], ClassB) << ": mask = " - << "0x" << utohexstr(mask) << "ULL"; - if (PtrArgNum >= 0) - OS << "; PtrArgNum = " << PtrArgNum; - if (HasConstPtr) - OS << "; HasConstPtr = true"; - OS << "; break;\n"; + std::pair I = OverloadMap.insert(std::make_pair( + MangleName(name, TypeVec[si], ClassB), OverloadInfo())); + OverloadInfo &Record = I.first->second; + if (!I.second) + assert(Record.PtrArgNum == PtrArgNum && + Record.HasConstPtr == HasConstPtr); + Record.Mask |= mask; + Record.PtrArgNum = PtrArgNum; + Record.HasConstPtr = HasConstPtr; } if (qmask) { - OS << "case NEON::BI__builtin_neon_"; - OS << MangleName(name, TypeVec[qi], ClassB) << ": mask = " - << "0x" << utohexstr(qmask) << "ULL"; - if (PtrArgNum >= 0) - OS << "; PtrArgNum = " << PtrArgNum; - if (HasConstPtr) - OS << "; HasConstPtr = true"; - OS << "; break;\n"; + std::pair I = OverloadMap.insert(std::make_pair( + MangleName(name, TypeVec[qi], ClassB), OverloadInfo())); + OverloadInfo &Record = I.first->second; + if (!I.second) + assert(Record.PtrArgNum == PtrArgNum && + Record.HasConstPtr == HasConstPtr); + Record.Mask |= qmask; + Record.PtrArgNum = PtrArgNum; + Record.HasConstPtr = HasConstPtr; } } + + for (OverloadIterator I = OverloadMap.begin(), E = OverloadMap.end(); I != E; + ++I) { + OverloadInfo &BuiltinOverloads = I->second; + OS << "case NEON::BI__builtin_neon_" << I->first << ": "; + OS << "mask = " << "0x" << utohexstr(BuiltinOverloads.Mask) << "ULL"; + if (BuiltinOverloads.PtrArgNum >= 0) + OS << "; PtrArgNum = " << BuiltinOverloads.PtrArgNum; + if (BuiltinOverloads.HasConstPtr) + OS << "; HasConstPtr = true"; + OS << "; break;\n"; + } + OS << "#endif\n\n"; } @@ -3248,41 +3232,14 @@ void NeonEmitter::genBuiltinsDef(raw_ostream &OS) { void NeonEmitter::runHeader(raw_ostream &OS) { std::vector RV = Records.getAllDerivedDefinitions("Inst"); - // build a map of AArch64 intriniscs to be used in uniqueness checks. - StringMap A64IntrinsicMap; - for (unsigned i = 0, e = RV.size(); i != e; ++i) { - Record *R = RV[i]; - - bool isA64 = R->getValueAsBit("isA64"); - if (!isA64) - continue; - - ClassKind CK = ClassNone; - if (R->getSuperClasses().size() >= 2) - CK = ClassMap[R->getSuperClasses()[1]]; - - std::string Name = R->getValueAsString("Name"); - std::string Proto = R->getValueAsString("Prototype"); - std::string Rename = Name + "@" + Proto; - if (A64IntrinsicMap.count(Rename)) - continue; - A64IntrinsicMap[Rename] = CK; - } - // Generate shared BuiltinsXXX.def genBuiltinsDef(OS); // Generate ARM overloaded type checking code for SemaChecking.cpp - genOverloadTypeCheckCode(OS, A64IntrinsicMap, false); - - // Generate AArch64 overloaded type checking code for SemaChecking.cpp - genOverloadTypeCheckCode(OS, A64IntrinsicMap, true); + genOverloadTypeCheckCode(OS); // Generate ARM range checking code for shift/lane immediates. - genIntrinsicRangeCheckCode(OS, A64IntrinsicMap, false); - - // Generate the AArch64 range checking code for shift/lane immediates. - genIntrinsicRangeCheckCode(OS, A64IntrinsicMap, true); + genIntrinsicRangeCheckCode(OS); } /// GenTest - Write out a test for the intrinsic specified by the name and -- 2.40.0