r246468 - Pull the target attribute parsing out of CGCall and onto TargetInfo.

Eric Christopher via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 31 16:36:24 PDT 2015


On Mon, Aug 31, 2015 at 4:34 PM Richard Smith <richard at metafoo.co.uk> wrote:

> On Mon, Aug 31, 2015 at 11:39 AM, Eric Christopher via cfe-commits <
> cfe-commits at 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 at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150831/90b4c2e7/attachment.html>


More information about the cfe-commits mailing list