[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-commits
Subject: Re: r246468 - Pull the target attribute parsing out of CGCall and onto TargetInfo.
From: Richard Smith via cfe-commits <cfe-commits () lists ! llvm ! org>
Date: 2015-08-31 23:34:59
Message-ID: CAOfiQqm470k2X44V-p=rsizcShuemQ5hG+VMwRzoYO2dUXR2+Q () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
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?
> +
> /// \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
>
[Attachment #5 (text/html)]
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Aug 31, 2015 \
at 11:39 AM, Eric Christopher via cfe-commits <span dir="ltr"><<a \
href="mailto:cfe-commits@lists.llvm.org" \
target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">Author: echristo<br>
Date: Mon Aug 31 13:39:22 2015<br>
New Revision: 246468<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=246468&view=rev" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=246468&view=rev</a><br>
Log:<br>
Pull the target attribute parsing out of CGCall and onto TargetInfo.<br>
<br>
Also:<br>
- Add a typedef to make working with the result easier.<br>
- Update callers to use the new function.<br>
- Make initFeatureMap out of line.<br>
<br>
Modified:<br>
cfe/trunk/include/clang/Basic/TargetInfo.h<br>
cfe/trunk/lib/Basic/TargetInfo.cpp<br>
cfe/trunk/lib/CodeGen/CGCall.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Basic/TargetInfo.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/TargetInfo.h?rev=246468&r1=246467&r2=246468&view=diff" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/include \
/clang/Basic/TargetInfo.h?rev=246468&r1=246467&r2=246468&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/include/clang/Basic/TargetInfo.h (original)<br>
+++ cfe/trunk/include/clang/Basic/TargetInfo.h Mon Aug 31 13:39:22 2015<br>
@@ -15,7 +15,9 @@<br>
#ifndef LLVM_CLANG_BASIC_TARGETINFO_H<br>
#define LLVM_CLANG_BASIC_TARGETINFO_H<br>
<br>
+#include "clang/AST/Attr.h"<br>
#include "clang/Basic/AddressSpaces.h"<br>
+#include "clang/Basic/Attributes.h"<br>
#include "clang/Basic/LLVM.h"<br>
#include "clang/Basic/Specifiers.h"<br>
#include "clang/Basic/TargetCXXABI.h"<br>
@@ -740,21 +742,18 @@ public:<br>
/// language options which change the target configuration.<br>
virtual void adjust(const LangOptions &Opts);<br>
<br>
+ /// \brief Parse a __target__ attribute and get the cpu/feature strings<br>
+ /// out of it for later use.<br>
+ typedef std::pair<StringRef, std::vector<std::string>> \
ParsedTargetAttr;<br> + ParsedTargetAttr parseTargetAttr(const TargetAttr *TA) \
const;<br></blockquote><div><br></div><div>Could you pass in the TargetAttr's \
features string here instead of the TargetAttr itself?</div><div> </div><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"> +<br>
/// \brief Initialize the map with the default set of target features for \
the<br>
/// CPU this should include all legal feature strings on the target.<br>
///<br>
/// \return False on error (invalid features).<br>
virtual bool initFeatureMap(llvm::StringMap<bool> &Features,<br>
DiagnosticsEngine &Diags, \
StringRef CPU,<br>
- std::vector<std::string> \
&FeatureVec) const {<br>
- for (const auto &F : FeatureVec) {<br>
- const char *Name = F.c_str();<br>
- // Apply the feature via the target.<br>
- bool Enabled = Name[0] == '+';<br>
- setFeatureEnabled(Features, Name + 1, Enabled);<br>
- }<br>
- return true;<br>
- }<br>
+ std::vector<std::string> \
&FeatureVec) const;<br> <br>
/// \brief Get the ABI currently in use.<br>
virtual StringRef getABI() const { return StringRef(); }<br>
<br>
Modified: cfe/trunk/lib/Basic/TargetInfo.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/TargetInfo.cpp?rev=246468&r1=246467&r2=246468&view=diff" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/TargetInfo.cpp?rev=246468&r1=246467&r2=246468&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Basic/TargetInfo.cpp (original)<br>
+++ cfe/trunk/lib/Basic/TargetInfo.cpp Mon Aug 31 13:39:22 2015<br>
@@ -650,3 +650,50 @@ bool TargetInfo::validateInputConstraint<br>
<br>
return true;<br>
}<br>
+<br>
+bool TargetInfo::initFeatureMap(llvm::StringMap<bool> &Features,<br>
+ DiagnosticsEngine &Diags, \
StringRef CPU,<br> + \
std::vector<std::string> &FeatureVec) const {<br> + for (const auto \
&F : FeatureVec) {<br> + const char *Name = F.c_str();<br>
+ // Apply the feature via the target.<br>
+ bool Enabled = Name[0] == '+';<br>
+ setFeatureEnabled(Features, Name + 1, Enabled);<br>
+ }<br>
+ return true;<br>
+}<br>
+<br>
+TargetInfo::ParsedTargetAttr<br>
+TargetInfo::parseTargetAttr(const TargetAttr *TA) const {<br>
+ std::pair<StringRef, std::vector<std::string>> RetPair;<br>
+<br>
+ // Grab the target attribute string.<br>
+ StringRef FeaturesStr = TA->getFeatures();<br>
+ SmallVector<StringRef, 1> AttrFeatures;<br>
+ FeaturesStr.split(AttrFeatures, ",");<br>
+<br>
+ // Grab the various features and prepend a "+" to turn on the feature \
to<br> + // the backend and add them to our existing set of features.<br>
+ for (auto &Feature : AttrFeatures) {<br>
+ // Go ahead and trim whitespace rather than either erroring or<br>
+ // accepting it weirdly.<br>
+ Feature = Feature.trim();<br>
+<br>
+ // While we're here iterating check for a different target cpu.<br>
+ if (Feature.startswith("arch="))<br>
+ RetPair.first = Feature.split("=").second.trim();<br>
+ else if (Feature.startswith("tune="))<br>
+ // We don't support cpu tuning this way currently.<br>
+ ;<br>
+ else if (Feature.startswith("fpmath="))<br>
+ // TODO: Support the fpmath option this way. It will require checking<br>
+ // overall feature validity for the function with the rest of the<br>
+ // attributes on the function.<br>
+ ;<br>
+ else if (Feature.startswith("no-"))<br>
+ RetPair.second.push_back("-" + \
Feature.split("-").second.str());<br> + else<br>
+ RetPair.second.push_back("+" + Feature.str());<br>
+ }<br>
+ return RetPair;<br>
+}<br>
<br>
Modified: cfe/trunk/lib/CodeGen/CGCall.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=246468&r1=246467&r2=246468&view=diff" \
rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGCall.cpp?rev=246468&r1=246467&r2=246468&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/CodeGen/CGCall.cpp (original)<br>
+++ cfe/trunk/lib/CodeGen/CGCall.cpp Mon Aug 31 13:39:22 2015<br>
@@ -1499,45 +1499,19 @@ void CodeGenModule::ConstructAttributeLi<br>
const FunctionDecl *FD = \
dyn_cast_or_null<FunctionDecl>(TargetDecl);<br> if (FD && \
FD->getAttr<TargetAttr>()) {<br> llvm::StringMap<bool> \
FeatureMap;<br>
- const auto *TD = FD->getAttr<TargetAttr>();<br>
<br>
- // Make a copy of the features as passed on the command line.<br>
- std::vector<std::string> FnFeatures =<br>
- getTarget().getTargetOpts().FeaturesAsWritten;<br>
-<br>
- // Grab the target attribute string.<br>
- StringRef FeaturesStr = TD->getFeatures();<br>
- SmallVector<StringRef, 1> AttrFeatures;<br>
- FeaturesStr.split(AttrFeatures, ",");<br>
-<br>
- // Grab the various features and prepend a "+" to turn on the \
feature to<br>
- // the backend and add them to our existing set of features.<br>
- for (auto &Feature : AttrFeatures) {<br>
- // Go ahead and trim whitespace rather than either erroring or<br>
- // accepting it weirdly.<br>
- Feature = Feature.trim();<br>
-<br>
- // While we're here iterating check for a different target cpu.<br>
- if (Feature.startswith("arch="))<br>
- TargetCPU = Feature.split("=").second.trim();<br>
- else if (Feature.startswith("tune="))<br>
- // We don't support cpu tuning this way currently.<br>
- ;<br>
- else if (Feature.startswith("fpmath="))<br>
- // TODO: Support the fpmath option this way. It will require \
checking<br>
- // overall feature validity for the function with the rest of the<br>
- // attributes on the function.<br>
- ;<br>
- else if (Feature.startswith("no-"))<br>
- FnFeatures.push_back("-" + \
Feature.split("-").second.str());<br>
- else<br>
- FnFeatures.push_back("+" + Feature.str());<br>
- }<br>
// Now populate the feature map, first with the TargetCPU which is \
either<br>
// the default or a new one from the target attribute string. Then \
we'll<br>
// use the passed in features (FeaturesAsWritten) along with the new \
ones<br> // from the attribute.<br>
- getTarget().initFeatureMap(FeatureMap, Diags, TargetCPU, FnFeatures);<br>
+ TargetInfo::ParsedTargetAttr PTA =<br>
+ getTarget().parseTargetAttr(FD->getAttr<TargetAttr>());<br>
+ if (PTA.first != "")<br>
+ TargetCPU = PTA.first;<br>
+ PTA.second.insert(PTA.second.begin(),<br>
+ \
getTarget().getTargetOpts().FeaturesAsWritten.begin(),<br> + \
getTarget().getTargetOpts().FeaturesAsWritten.end());<br> + \
getTarget().initFeatureMap(FeatureMap, Diags, TargetCPU, PTA.second);<br> <br>
// Produce the canonical string for this set of features.<br>
std::vector<std::string> Features;<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" \
target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br> \
</blockquote></div><br></div></div>
[Attachment #6 (text/plain)]
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic