<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, Aug 31, 2015 at 4:34 PM Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><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></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Could you pass in the TargetAttr's features string here instead of the TargetAttr itself?</div></div></div></div></blockquote><div><br></div><div>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.</div><div><br></div><div>Thoughts?</div><div><br></div><div>-eric</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><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" target="_blank">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></div></div></blockquote></div></div>