Index: include/clang/Basic/Attr.td =================================================================== --- include/clang/Basic/Attr.td +++ include/clang/Basic/Attr.td @@ -1802,11 +1802,18 @@ let Subjects = SubjectList<[Function], ErrorDiag>; let Documentation = [TargetDocs]; let AdditionalMembers = [{ - typedef std::pair, StringRef> ParsedTargetAttr; + struct ParsedTargetAttr { + std::vector Features; + StringRef Architecture; + bool DuplicateArchitecture = false; + }; ParsedTargetAttr parse() const { + return parse(getFeaturesStr()); + } + static ParsedTargetAttr parse(StringRef Features) { ParsedTargetAttr Ret; SmallVector AttrFeatures; - getFeaturesStr().split(AttrFeatures, ","); + Features.split(AttrFeatures, ","); // Grab the various features and prepend a "+" to turn on the feature to // the backend and add them to our existing set of features. @@ -1823,12 +1830,15 @@ continue; // While we're here iterating check for a different target cpu. - if (Feature.startswith("arch=")) - Ret.second = Feature.split("=").second.trim(); - else if (Feature.startswith("no-")) - Ret.first.push_back("-" + Feature.split("-").second.str()); + if (Feature.startswith("arch=")) { + if (!Ret.Architecture.empty()) + Ret.DuplicateArchitecture = true; + else + Ret.Architecture = Feature.split("=").second.trim(); + } else if (Feature.startswith("no-")) + Ret.Features.push_back("-" + Feature.split("-").second.str()); else - Ret.first.push_back("+" + Feature.str()); + Ret.Features.push_back("+" + Feature.str()); } return Ret; } Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -2387,8 +2387,9 @@ def err_attribute_requires_opencl_version : Error< "%0 attribute requires OpenCL version %1%select{| or above}2">; def warn_unsupported_target_attribute - : Warning<"Ignoring unsupported '%0' in the target attribute string">, - InGroup; + : Warning<"ignoring %select{unsupported|duplicate}0" + "%select{| architecture}1 '%2' in the target attribute string">, + InGroup; def err_attribute_unsupported : Error<"%0 attribute is not supported for this target">; // The err_*_attribute_argument_not_int are separate because they're used by Index: include/clang/Basic/TargetInfo.h =================================================================== --- include/clang/Basic/TargetInfo.h +++ include/clang/Basic/TargetInfo.h @@ -869,6 +869,11 @@ return false; } + /// brief Determine whether this TargetInfo supports the given CPU name. + virtual bool isValidCPUName(StringRef Name) const { + return false; + } + /// \brief Use the specified ABI. /// /// \return False on error (invalid ABI name). @@ -891,6 +896,11 @@ Features[Name] = Enabled; } + /// \brief Determine whether this TargetInfo supports the given feature. + virtual bool isValidFeatureName(StringRef Feature) const { + return false; + } + /// \brief Perform initialization based on the user configured /// set of features (e.g., +sse4). /// Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -3271,7 +3271,7 @@ unsigned ArgNum, StringRef &Str, SourceLocation *ArgLocation = nullptr); bool checkSectionName(SourceLocation LiteralLoc, StringRef Str); - void checkTargetAttr(SourceLocation LiteralLoc, StringRef Str); + bool checkTargetAttr(SourceLocation LiteralLoc, StringRef Str); bool checkMSInheritanceAttrOnDefinition( CXXRecordDecl *RD, SourceRange Range, bool BestCase, MSInheritanceAttr::Spelling SemanticSpelling); Index: lib/Basic/Targets.cpp =================================================================== --- lib/Basic/Targets.cpp +++ lib/Basic/Targets.cpp @@ -963,8 +963,8 @@ // 401, 403, 405, 405fp, 440fp, 464, 464fp, 476, 476fp, 505, 740, 801, // 821, 823, 8540, 8548, e300c2, e300c3, e500mc64, e6500, 860, cell, // titan, rs64. - bool setCPU(const std::string &Name) override { - bool CPUKnown = llvm::StringSwitch(Name) + bool isValidCPUName(StringRef Name) const override { + return llvm::StringSwitch(Name) .Case("generic", true) .Case("440", true) .Case("450", true) @@ -1014,10 +1014,12 @@ .Case("powerpc64le", true) .Case("ppc64le", true) .Default(false); + } + bool setCPU(const std::string &Name) override { + bool CPUKnown = isValidCPUName(Name); if (CPUKnown) CPU = Name; - return CPUKnown; } @@ -2006,6 +2008,9 @@ // FIXME: implement return TargetInfo::CharPtrBuiltinVaList; } + bool isValidCPUName(StringRef Name) const override { + return StringToCudaArch(Name) != CudaArch::UNKNOWN; + } bool setCPU(const std::string &Name) override { GPU = StringToCudaArch(Name); return GPU != CudaArch::UNKNOWN; @@ -2367,6 +2372,13 @@ .Default(GK_NONE); } + bool isValidCPUName(StringRef Name) const override { + if (getTriple().getArch() == llvm::Triple::amdgcn) + return GK_NONE != parseAMDGCNName(Name); + else + return GK_NONE != parseR600Name(Name); + } + bool setCPU(const std::string &Name) override { if (getTriple().getArch() == llvm::Triple::amdgcn) GPU = parseAMDGCNName(Name); @@ -2873,6 +2885,87 @@ //@} } CPU = CK_Generic; + bool checkCPUKind(CPUKind Kind) const { + // Perform any per-CPU checks necessary to determine if this CPU is + // acceptable. + // FIXME: This results in terrible diagnostics. Clang just says the CPU is + // invalid without explaining *why*. + switch (Kind) { + case CK_Generic: + // No processor selected! + return false; + + case CK_i386: + case CK_i486: + case CK_WinChipC6: + case CK_WinChip2: + case CK_C3: + case CK_i586: + case CK_Pentium: + case CK_PentiumMMX: + case CK_i686: + case CK_PentiumPro: + case CK_Pentium2: + case CK_Pentium3: + case CK_Pentium3M: + case CK_PentiumM: + case CK_Yonah: + case CK_C3_2: + case CK_Pentium4: + case CK_Pentium4M: + case CK_Lakemont: + case CK_Prescott: + case CK_K6: + case CK_K6_2: + case CK_K6_3: + case CK_Athlon: + case CK_AthlonThunderbird: + case CK_Athlon4: + case CK_AthlonXP: + case CK_AthlonMP: + case CK_Geode: + // Only accept certain architectures when compiling in 32-bit mode. + if (getTriple().getArch() != llvm::Triple::x86) + return false; + + // Fallthrough + case CK_Nocona: + case CK_Core2: + case CK_Penryn: + case CK_Bonnell: + case CK_Silvermont: + case CK_Goldmont: + case CK_Nehalem: + case CK_Westmere: + case CK_SandyBridge: + case CK_IvyBridge: + case CK_Haswell: + case CK_Broadwell: + case CK_SkylakeClient: + case CK_SkylakeServer: + case CK_Cannonlake: + case CK_KNL: + case CK_Athlon64: + case CK_Athlon64SSE3: + case CK_AthlonFX: + case CK_K8: + case CK_K8SSE3: + case CK_Opteron: + case CK_OpteronSSE3: + case CK_AMDFAM10: + case CK_BTVER1: + case CK_BTVER2: + case CK_BDVER1: + case CK_BDVER2: + case CK_BDVER3: + case CK_BDVER4: + case CK_ZNVER1: + case CK_x86_64: + return true; + } + llvm_unreachable("Unhandled CPU kind"); + } + CPUKind getCPUKind(StringRef CPU) const { return llvm::StringSwitch(CPU) .Case("i386", CK_i386) @@ -3053,6 +3146,7 @@ initFeatureMap(llvm::StringMap &Features, DiagnosticsEngine &Diags, StringRef CPU, const std::vector &FeaturesVec) const override; + bool isValidFeatureName(StringRef Name) const override; bool hasFeature(StringRef Feature) const override; bool handleTargetFeatures(std::vector &Features, DiagnosticsEngine &Diags) override; @@ -3066,87 +3160,13 @@ return "no-mmx"; return ""; } - bool setCPU(const std::string &Name) override { - CPU = getCPUKind(Name); - // Perform any per-CPU checks necessary to determine if this CPU is - // acceptable. - // FIXME: This results in terrible diagnostics. Clang just says the CPU is - // invalid without explaining *why*. - switch (CPU) { - case CK_Generic: - // No processor selected! - return false; - - case CK_i386: - case CK_i486: - case CK_WinChipC6: - case CK_WinChip2: - case CK_C3: - case CK_i586: - case CK_Pentium: - case CK_PentiumMMX: - case CK_i686: - case CK_PentiumPro: - case CK_Pentium2: - case CK_Pentium3: - case CK_Pentium3M: - case CK_PentiumM: - case CK_Yonah: - case CK_C3_2: - case CK_Pentium4: - case CK_Pentium4M: - case CK_Lakemont: - case CK_Prescott: - case CK_K6: - case CK_K6_2: - case CK_K6_3: - case CK_Athlon: - case CK_AthlonThunderbird: - case CK_Athlon4: - case CK_AthlonXP: - case CK_AthlonMP: - case CK_Geode: - // Only accept certain architectures when compiling in 32-bit mode. - if (getTriple().getArch() != llvm::Triple::x86) - return false; + bool isValidCPUName(StringRef Name) const override { + return checkCPUKind(getCPUKind(Name)); + } - // Fallthrough - case CK_Nocona: - case CK_Core2: - case CK_Penryn: - case CK_Bonnell: - case CK_Silvermont: - case CK_Goldmont: - case CK_Nehalem: - case CK_Westmere: - case CK_SandyBridge: - case CK_IvyBridge: - case CK_Haswell: - case CK_Broadwell: - case CK_SkylakeClient: - case CK_SkylakeServer: - case CK_Cannonlake: - case CK_KNL: - case CK_Athlon64: - case CK_Athlon64SSE3: - case CK_AthlonFX: - case CK_K8: - case CK_K8SSE3: - case CK_Opteron: - case CK_OpteronSSE3: - case CK_AMDFAM10: - case CK_BTVER1: - case CK_BTVER2: - case CK_BDVER1: - case CK_BDVER2: - case CK_BDVER3: - case CK_BDVER4: - case CK_ZNVER1: - case CK_x86_64: - return true; - } - llvm_unreachable("Unhandled CPU kind"); + bool setCPU(const std::string &Name) override { + return checkCPUKind(CPU = getCPUKind(Name)); } bool setFPMath(StringRef Name) override; @@ -4239,6 +4259,68 @@ Builder.defineMacro("__SIZEOF_FLOAT128__", "16"); } +bool X86TargetInfo::isValidFeatureName(StringRef Name) const { + return llvm::StringSwitch(Name) + .Case("aes", true) + .Case("avx", true) + .Case("avx2", true) + .Case("avx512f", true) + .Case("avx512cd", true) + .Case("avx512vpopcntdq", true) + .Case("avx512er", true) + .Case("avx512pf", true) + .Case("avx512dq", true) + .Case("avx512bw", true) + .Case("avx512vl", true) + .Case("avx512vbmi", true) + .Case("avx512ifma", true) + .Case("bmi", true) + .Case("bmi2", true) + .Case("clflushopt", true) + .Case("clwb", true) + .Case("clzero", true) + .Case("cx16", true) + .Case("f16c", true) + .Case("fma", true) + .Case("fma4", true) + .Case("fsgsbase", true) + .Case("fxsr", true) + .Case("lwp", true) + .Case("lzcnt", true) + .Case("mm3dnow", true) + .Case("mm3dnowa", true) + .Case("mmx", true) + .Case("movbe", true) + .Case("mpx", true) + .Case("pclmul", true) + .Case("pku", true) + .Case("popcnt", true) + .Case("prefetchwt1", true) + .Case("prfchw", true) + .Case("rdrnd", true) + .Case("rdseed", true) + .Case("rtm", true) + .Case("sgx", true) + .Case("sha", true) + .Case("sse", true) + .Case("sse2", true) + .Case("sse3", true) + .Case("ssse3", true) + .Case("sse4.1", true) + .Case("sse4.2", true) + .Case("sse4a", true) + .Case("tbm", true) + .Case("x86", true) + .Case("x86_32", true) + .Case("x86_64", true) + .Case("xop", true) + .Case("xsave", true) + .Case("xsavec", true) + .Case("xsaves", true) + .Case("xsaveopt", true) + .Default(false); +} + bool X86TargetInfo::hasFeature(StringRef Feature) const { return llvm::StringSwitch(Feature) .Case("aes", HasAES) @@ -4265,6 +4347,7 @@ .Case("fma4", XOPLevel >= FMA4) .Case("fsgsbase", HasFSGSBASE) .Case("fxsr", HasFXSR) + .Case("lwp", HasLWP) .Case("lzcnt", HasLZCNT) .Case("mm3dnow", MMX3DNowLevel >= AMD3DNow) .Case("mm3dnowa", MMX3DNowLevel >= AMD3DNowAthlon) @@ -4289,7 +4372,6 @@ .Case("sse4.2", SSELevel >= SSE42) .Case("sse4a", XOPLevel >= SSE4A) .Case("tbm", HasTBM) - .Case("lwp", HasLWP) .Case("x86", true) .Case("x86_32", getTriple().getArch() == llvm::Triple::x86) .Case("x86_64", getTriple().getArch() == llvm::Triple::x86_64) @@ -5634,6 +5716,11 @@ .Default(false); } + bool isValidCPUName(StringRef Name) const override { + return Name == "generic" || + llvm::ARM::parseCPUArch(Name) != llvm::ARM::AK_INVALID; + } + bool setCPU(const std::string &Name) override { if (Name != "generic") setArchInfo(llvm::ARM::parseCPUArch(Name)); @@ -6319,12 +6406,16 @@ return true; } - bool setCPU(const std::string &Name) override { + bool isValidCPUName(StringRef Name) const override { return Name == "generic" || llvm::AArch64::parseCPUArch(Name) != static_cast(llvm::AArch64::ArchKind::AK_INVALID); } + bool setCPU(const std::string &Name) override { + return isValidCPUName(Name); + } + void getTargetDefinesARMV81A(const LangOptions &Opts, MacroBuilder &Builder) const { Builder.defineMacro("__ARM_FEATURE_QRDMX", "1"); @@ -6829,8 +6920,12 @@ .Default(nullptr); } + bool isValidCPUName(StringRef Name) const override { + return getHexagonCPUSuffix(Name); + } + bool setCPU(const std::string &Name) override { - if (!getHexagonCPUSuffix(Name)) + if (!isValidCPUName(Name)) return false; CPU = Name; return true; @@ -7010,6 +7105,12 @@ } } + bool isValidCPUName(StringRef Name) const override { + return llvm::StringSwitch(Name) + .Case("v11", true) + .Default(false); + } + bool setCPU(const std::string &Name) override { CPU = llvm::StringSwitch(Name) .Case("v11", CK_V11) @@ -7248,6 +7349,10 @@ .Default(CK_GENERIC); } + bool isValidCPUName(StringRef Name) const override { + return getCPUKind(Name) != CK_GENERIC; + } + bool setCPU(const std::string &Name) override { CPU = getCPUKind(Name); return CPU != CK_GENERIC; @@ -7424,6 +7529,10 @@ } } + bool isValidCPUName(StringRef Name) const override { + return getCPUGeneration(SparcTargetInfo::getCPUKind(Name)) == CG_V9; + } + bool setCPU(const std::string &Name) override { if (!SparcTargetInfo::setCPU(Name)) return false; @@ -7506,6 +7615,11 @@ .Cases("arch12", "z14", 12) .Default(-1); } + + bool isValidCPUName(StringRef Name) const override { + return getISARevision(Name) != -1; + } + bool setCPU(const std::string &Name) override { CPU = Name; ISARevision = getISARevision(CPU); @@ -7886,8 +8000,12 @@ return false; } + bool isValidCPUName(StringRef Name) const override { + return Name == "nios2r1" || Name == "nios2r2"; + } + bool setCPU(const std::string &Name) override { - if (Name == "nios2r1" || Name == "nios2r2") { + if (isValidCPUName(Name)) { CPU = Name; return true; } @@ -8154,8 +8272,7 @@ SizeType = UnsignedInt; } - bool setCPU(const std::string &Name) override { - CPU = Name; + bool isValidCPUName(StringRef Name) const override { return llvm::StringSwitch(Name) .Case("mips1", true) .Case("mips2", true) @@ -8176,6 +8293,12 @@ .Case("p5600", true) .Default(false); } + + bool setCPU(const std::string &Name) override { + CPU = Name; + return isValidCPUName(Name); + } + const std::string& getCPU() const { return CPU; } bool initFeatureMap(llvm::StringMap &Features, DiagnosticsEngine &Diags, @@ -8733,13 +8856,16 @@ } return true; } - bool setCPU(const std::string &Name) final { + bool isValidCPUName(StringRef Name) const final { return llvm::StringSwitch(Name) .Case("mvp", true) .Case("bleeding-edge", true) .Case("generic", true) .Default(false); } + bool setCPU(const std::string &Name) final { + return isValidCPUName(Name); + } ArrayRef getTargetBuiltins() const final { return llvm::makeArrayRef(BuiltinInfo, clang::WebAssembly::LastTSBuiltin - Builtin::FirstTSBuiltin); @@ -9459,7 +9585,7 @@ : TargetInfo::getLeastIntTypeByWidth(BitWidth, IsSigned); } - bool setCPU(const std::string &Name) override { + bool isValidCPUName(StringRef Name) const override { bool IsFamily = llvm::StringSwitch(Name) .Case("avr1", true) .Case("avr2", true) @@ -9481,16 +9607,18 @@ .Case("avrtiny", true) .Default(false); - if (IsFamily) this->CPU = Name; - bool IsMCU = std::find_if(AVRMcus.begin(), AVRMcus.end(), [&](const MCUInfo &Info) { return Info.Name == Name; }) != AVRMcus.end(); - - if (IsMCU) this->CPU = Name; - return IsFamily || IsMCU; } + bool setCPU(const std::string &Name) override { + bool isValid = isValidCPUName(Name); + if (isValid) + CPU = Name; + return isValid; + } + protected: std::string CPU; }; Index: lib/CodeGen/CGCall.cpp =================================================================== --- lib/CodeGen/CGCall.cpp +++ lib/CodeGen/CGCall.cpp @@ -1877,8 +1877,8 @@ // the function. const auto *TD = FD->getAttr(); TargetAttr::ParsedTargetAttr ParsedAttr = TD->parse(); - if (ParsedAttr.second != "") - TargetCPU = ParsedAttr.second; + if (ParsedAttr.Architecture != "") + TargetCPU = ParsedAttr.Architecture; if (TargetCPU != "") FuncAttrs.addAttribute("target-cpu", TargetCPU); if (!Features.empty()) { Index: lib/CodeGen/CodeGenModule.cpp =================================================================== --- lib/CodeGen/CodeGenModule.cpp +++ lib/CodeGen/CodeGenModule.cpp @@ -4499,18 +4499,19 @@ // Make a copy of the features as passed on the command line into the // beginning of the additional features from the function to override. - ParsedAttr.first.insert(ParsedAttr.first.begin(), + ParsedAttr.Features.insert(ParsedAttr.Features.begin(), Target.getTargetOpts().FeaturesAsWritten.begin(), Target.getTargetOpts().FeaturesAsWritten.end()); - if (ParsedAttr.second != "") - TargetCPU = ParsedAttr.second; + if (ParsedAttr.Architecture != "") + TargetCPU = ParsedAttr.Architecture ; // Now populate the feature map, first with the TargetCPU which is either // the default or a new one from the target attribute string. Then we'll use // the passed in features (FeaturesAsWritten) along with the new ones from // the attribute. - Target.initFeatureMap(FeatureMap, getDiags(), TargetCPU, ParsedAttr.first); + Target.initFeatureMap(FeatureMap, getDiags(), TargetCPU, + ParsedAttr.Features); } else { Target.initFeatureMap(FeatureMap, getDiags(), TargetCPU, Target.getTargetOpts().Features); Index: lib/Sema/SemaDeclAttr.cpp =================================================================== --- lib/Sema/SemaDeclAttr.cpp +++ lib/Sema/SemaDeclAttr.cpp @@ -2993,22 +2993,42 @@ D->addAttr(NewAttr); } -// Check for things we'd like to warn about, no errors or validation for now. -// TODO: Validation should use a backend target library that specifies -// the allowable subtarget features and cpus. We could use something like a -// TargetCodeGenInfo hook here to do validation. -void Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) { +// Check for things we'd like to warn about. Multiversioning issues are +// handled later in the process, once we know how many exist. +bool Sema::checkTargetAttr(SourceLocation LiteralLoc, StringRef AttrStr) { + // Error message enums. Enum vs enum class since the scope is limited, + // and it otherwise simplifies the error messages. + enum FirstParam { unsupported, duplicate}; + enum SecondParam { None, architecture }; for (auto Str : {"tune=", "fpmath="}) if (AttrStr.find(Str) != StringRef::npos) - Diag(LiteralLoc, diag::warn_unsupported_target_attribute) << Str; + return Diag(LiteralLoc, diag::warn_unsupported_target_attribute) << unsupported << None << Str; + + TargetAttr::ParsedTargetAttr ParsedAttrs = TargetAttr::parse(AttrStr); + if (ParsedAttrs.DuplicateArchitecture) + return Diag(LiteralLoc, diag::warn_unsupported_target_attribute) << duplicate << None << "arch="; + + if (!ParsedAttrs.Architecture.empty() && + !Context.getTargetInfo().isValidCPUName(ParsedAttrs.Architecture)) + return Diag(LiteralLoc, diag::warn_unsupported_target_attribute) << unsupported << architecture + << ParsedAttrs.Architecture; + + for (const auto &Feature : ParsedAttrs.Features) { + auto CurFeature = StringRef(Feature).drop_front(); // remove + or -. + if (!Context.getTargetInfo().isValidFeatureName(CurFeature)) + return Diag(LiteralLoc, diag::warn_unsupported_target_attribute) << unsupported << None + << CurFeature; + } + + return true; } static void handleTargetAttr(Sema &S, Decl *D, const AttributeList &Attr) { StringRef Str; SourceLocation LiteralLoc; - if (!S.checkStringLiteralArgumentAttr(Attr, 0, Str, &LiteralLoc)) + if (!S.checkStringLiteralArgumentAttr(Attr, 0, Str, &LiteralLoc) || + !S.checkTargetAttr(LiteralLoc, Str)) return; - S.checkTargetAttr(LiteralLoc, Str); unsigned Index = Attr.getAttributeSpellingListIndex(); TargetAttr *NewAttr = ::new (S.Context) TargetAttr(Attr.getRange(), S.Context, Str, Index); Index: test/Sema/attr-target.c =================================================================== --- test/Sema/attr-target.c +++ test/Sema/attr-target.c @@ -2,7 +2,12 @@ int __attribute__((target("avx,sse4.2,arch=ivybridge"))) foo() { return 4; } int __attribute__((target())) bar() { return 4; } //expected-error {{'target' attribute takes one argument}} -int __attribute__((target("tune=sandybridge"))) baz() { return 4; } //expected-warning {{Ignoring unsupported 'tune=' in the target attribute string}} -int __attribute__((target("fpmath=387"))) walrus() { return 4; } //expected-warning {{Ignoring unsupported 'fpmath=' in the target attribute string}} +int __attribute__((target("tune=sandybridge"))) baz() { return 4; } //expected-warning {{ignoring unsupported 'tune=' in the target attribute string}} +int __attribute__((target("fpmath=387"))) walrus() { return 4; } //expected-warning {{ignoring unsupported 'fpmath=' in the target attribute string}} +int __attribute__((target("avx,sse4.2,arch=hiss"))) meow() { return 4; }//expected-warning {{ignoring unsupported architecture 'hiss' in the target attribute string}} +int __attribute__((target("woof"))) bark() { return 4; }//expected-warning {{ignoring unsupported 'woof' in the target attribute string}} +int __attribute__((target("arch="))) turtle() { return 4; } // no warning, same as saying 'nothing'. +int __attribute__((target("arch=hiss,arch=woof"))) pine_tree() { return 4; } //expected-warning {{ignoring duplicate 'arch=' in the target attribute string}} +