On Mon, Aug 31, 2015 at 4:34 PM Richard Smith <rich...@metafoo.co.uk> wrote:
> On Mon, Aug 31, 2015 at 11:39 AM, Eric Christopher via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: echristo >> Date: Mon Aug 31 13:39:22 2015 >> New Revision: 246468 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=246468&view=rev >> Log: >> Pull the target attribute parsing out of CGCall and onto TargetInfo. >> >> Also: >> - Add a typedef to make working with the result easier. >> - Update callers to use the new function. >> - Make initFeatureMap out of line. >> >> Modified: >> cfe/trunk/include/clang/Basic/TargetInfo.h >> cfe/trunk/lib/Basic/TargetInfo.cpp >> cfe/trunk/lib/CodeGen/CGCall.cpp >> >> Modified: cfe/trunk/include/clang/Basic/TargetInfo.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TargetInfo.h?rev=246468&r1=246467&r2=246468&view=diff >> >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/TargetInfo.h (original) >> +++ cfe/trunk/include/clang/Basic/TargetInfo.h Mon Aug 31 13:39:22 2015 >> @@ -15,7 +15,9 @@ >> #ifndef LLVM_CLANG_BASIC_TARGETINFO_H >> #define LLVM_CLANG_BASIC_TARGETINFO_H >> >> +#include "clang/AST/Attr.h" >> #include "clang/Basic/AddressSpaces.h" >> +#include "clang/Basic/Attributes.h" >> #include "clang/Basic/LLVM.h" >> #include "clang/Basic/Specifiers.h" >> #include "clang/Basic/TargetCXXABI.h" >> @@ -740,21 +742,18 @@ public: >> /// language options which change the target configuration. >> virtual void adjust(const LangOptions &Opts); >> >> + /// \brief Parse a __target__ attribute and get the cpu/feature strings >> + /// out of it for later use. >> + typedef std::pair<StringRef, std::vector<std::string>> >> ParsedTargetAttr; >> + ParsedTargetAttr parseTargetAttr(const TargetAttr *TA) const; >> > > Could you pass in the TargetAttr's features string here instead of the > TargetAttr itself? > I could. I'd also thought about just putting the parsing as a static metho on the attribute class since it doesn't need to know anything about it in general. Thoughts? -eric > > >> + >> /// \brief Initialize the map with the default set of target features >> for the >> /// CPU this should include all legal feature strings on the target. >> /// >> /// \return False on error (invalid features). >> virtual bool initFeatureMap(llvm::StringMap<bool> &Features, >> DiagnosticsEngine &Diags, StringRef CPU, >> - std::vector<std::string> &FeatureVec) >> const { >> - for (const auto &F : FeatureVec) { >> - const char *Name = F.c_str(); >> - // Apply the feature via the target. >> - bool Enabled = Name[0] == '+'; >> - setFeatureEnabled(Features, Name + 1, Enabled); >> - } >> - return true; >> - } >> + std::vector<std::string> &FeatureVec) >> const; >> >> /// \brief Get the ABI currently in use. >> virtual StringRef getABI() const { return StringRef(); } >> >> Modified: cfe/trunk/lib/Basic/TargetInfo.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/TargetInfo.cpp?rev=246468&r1=246467&r2=246468&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Basic/TargetInfo.cpp (original) >> +++ cfe/trunk/lib/Basic/TargetInfo.cpp Mon Aug 31 13:39:22 2015 >> @@ -650,3 +650,50 @@ bool TargetInfo::validateInputConstraint >> >> return true; >> } >> + >> +bool TargetInfo::initFeatureMap(llvm::StringMap<bool> &Features, >> + DiagnosticsEngine &Diags, StringRef CPU, >> + std::vector<std::string> &FeatureVec) >> const { >> + for (const auto &F : FeatureVec) { >> + const char *Name = F.c_str(); >> + // Apply the feature via the target. >> + bool Enabled = Name[0] == '+'; >> + setFeatureEnabled(Features, Name + 1, Enabled); >> + } >> + return true; >> +} >> + >> +TargetInfo::ParsedTargetAttr >> +TargetInfo::parseTargetAttr(const TargetAttr *TA) const { >> + std::pair<StringRef, std::vector<std::string>> RetPair; >> + >> + // Grab the target attribute string. >> + StringRef FeaturesStr = TA->getFeatures(); >> + SmallVector<StringRef, 1> AttrFeatures; >> + FeaturesStr.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. >> + for (auto &Feature : AttrFeatures) { >> + // Go ahead and trim whitespace rather than either erroring or >> + // accepting it weirdly. >> + Feature = Feature.trim(); >> + >> + // While we're here iterating check for a different target cpu. >> + if (Feature.startswith("arch=")) >> + RetPair.first = Feature.split("=").second.trim(); >> + else if (Feature.startswith("tune=")) >> + // We don't support cpu tuning this way currently. >> + ; >> + else if (Feature.startswith("fpmath=")) >> + // TODO: Support the fpmath option this way. It will require >> checking >> + // overall feature validity for the function with the rest of the >> + // attributes on the function. >> + ; >> + else if (Feature.startswith("no-")) >> + RetPair.second.push_back("-" + Feature.split("-").second.str()); >> + else >> + RetPair.second.push_back("+" + Feature.str()); >> + } >> + return RetPair; >> +} >> >> Modified: cfe/trunk/lib/CodeGen/CGCall.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=246468&r1=246467&r2=246468&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/CodeGen/CGCall.cpp (original) >> +++ cfe/trunk/lib/CodeGen/CGCall.cpp Mon Aug 31 13:39:22 2015 >> @@ -1499,45 +1499,19 @@ void CodeGenModule::ConstructAttributeLi >> const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(TargetDecl); >> if (FD && FD->getAttr<TargetAttr>()) { >> llvm::StringMap<bool> FeatureMap; >> - const auto *TD = FD->getAttr<TargetAttr>(); >> >> - // Make a copy of the features as passed on the command line. >> - std::vector<std::string> FnFeatures = >> - getTarget().getTargetOpts().FeaturesAsWritten; >> - >> - // Grab the target attribute string. >> - StringRef FeaturesStr = TD->getFeatures(); >> - SmallVector<StringRef, 1> AttrFeatures; >> - FeaturesStr.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. >> - for (auto &Feature : AttrFeatures) { >> - // Go ahead and trim whitespace rather than either erroring or >> - // accepting it weirdly. >> - Feature = Feature.trim(); >> - >> - // While we're here iterating check for a different target cpu. >> - if (Feature.startswith("arch=")) >> - TargetCPU = Feature.split("=").second.trim(); >> - else if (Feature.startswith("tune=")) >> - // We don't support cpu tuning this way currently. >> - ; >> - else if (Feature.startswith("fpmath=")) >> - // TODO: Support the fpmath option this way. It will require >> checking >> - // overall feature validity for the function with the rest of >> the >> - // attributes on the function. >> - ; >> - else if (Feature.startswith("no-")) >> - FnFeatures.push_back("-" + Feature.split("-").second.str()); >> - else >> - FnFeatures.push_back("+" + Feature.str()); >> - } >> // 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. >> - getTarget().initFeatureMap(FeatureMap, Diags, TargetCPU, >> FnFeatures); >> + TargetInfo::ParsedTargetAttr PTA = >> + getTarget().parseTargetAttr(FD->getAttr<TargetAttr>()); >> + if (PTA.first != "") >> + TargetCPU = PTA.first; >> + PTA.second.insert(PTA.second.begin(), >> + >> getTarget().getTargetOpts().FeaturesAsWritten.begin(), >> + >> getTarget().getTargetOpts().FeaturesAsWritten.end()); >> + getTarget().initFeatureMap(FeatureMap, Diags, TargetCPU, >> PTA.second); >> >> // Produce the canonical string for this set of features. >> std::vector<std::string> Features; >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits