<br><br><div class="gmail_quote">2011/9/22 Justin Holewinski <span dir="ltr"><<a href="mailto:justin.holewinski@gmail.com">justin.holewinski@gmail.com</a>></span><br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div class="gmail_quote"><div><div></div><div class="h5">On Thu, Sep 15, 2011 at 6:08 PM, Justin Holewinski <span dir="ltr"><<a href="mailto:justin.holewinski@gmail.com" target="_blank">justin.holewinski@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div><div></div><div><div>On Thu, Sep 15, 2011 at 1:45 PM, Matthieu Monrocq <span dir="ltr"><<a href="mailto:matthieu.monrocq@gmail.com" target="_blank">matthieu.monrocq@gmail.com</a>></span> wrote:</div>
</div></div><div class="gmail_quote"><div><div></div><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Date: Thu, 15 Sep 2011 15:34:14 +0100<br>
From: Tobias Grosser <<a href="mailto:tobias@grosser.es" target="_blank">tobias@grosser.es</a>><br>
Subject: Re: [cfe-commits] r139789 - /cfe/trunk/lib/Basic/Targets.cpp<br>
To: Justin Holewinski <<a href="mailto:justin.holewinski@gmail.com" target="_blank">justin.holewinski@gmail.com</a>><br>
Cc: <a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
Message-ID: <<a href="mailto:4E720CE6.9040001@grosser.es" target="_blank">4E720CE6.9040001@grosser.es</a>><br>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed<div><div></div><div><br>
<br>
On 09/15/2011 01:13 PM, Justin Holewinski wrote:<br>
> Author: jholewinski<br>
> Date: Thu Sep 15 07:13:38 2011<br>
> New Revision: 139789<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=139789&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=139789&view=rev</a><br>
> Log:<br>
> PTX: Define target options<br>
Hey Justin,<br>
<br>
this is nice, but you might shorten the code slightly.<br>
<br>
> Modified:<br>
> cfe/trunk/lib/Basic/Targets.cpp<br>
><br>
> Modified: cfe/trunk/lib/Basic/Targets.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=139789&r1=139788&r2=139789&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=139789&r1=139788&r2=139789&view=diff</a><br>
> ==============================================================================<br>
> --- cfe/trunk/lib/Basic/Targets.cpp (original)<br>
> +++ cfe/trunk/lib/Basic/Targets.cpp Thu Sep 15 07:13:38 2011<br>
> @@ -916,6 +916,10 @@<br>
> // FIXME: implement<br>
> return "typedef char* __builtin_va_list;";<br>
> }<br>
> +<br>
> + virtual bool setFeatureEnabled(llvm::StringMap<bool> &Features,<br>
> + const std::string&Name,<br>
> + bool Enabled) const;<br>
> };<br>
><br>
> const Builtin::Info PTXTargetInfo::BuiltinInfo[] = {<br>
> @@ -935,6 +939,91 @@<br>
> NumNames = llvm::array_lengthof(GCCRegNames);<br>
> }<br>
><br>
> + bool PTXTargetInfo::setFeatureEnabled(llvm::StringMap<bool> &Features,<br>
> + const std::string&Name,<br>
> + bool Enabled) const {<br>
> + if (Enabled) {<br>
> + if (Name == "double")<br>
> + Features["double"] = true;<br>
> + else if (Name == "no-fma")<br>
> + Features["no-fma"] = true;<br>
> + else if (Name == "compute10")<br>
> + Features["compute10"] = true;<br>
> + else if (Name == "compute11")<br>
> + Features["compute11"] = true;<br>
> + else if (Name == "compute12")<br>
> + Features["compute12"] = true;<br>
> + else if (Name == "compute13")<br>
> + Features["compute13"] = true;<br>
> + else if (Name == "compute20")<br>
> + Features["compute20"] = true;<br>
> + else if (Name == "ptx20")<br>
> + Features["ptx20"] = true;<br>
> + else if (Name == "ptx21")<br>
> + Features["ptx21"] = true;<br>
> + else if (Name == "ptx22")<br>
> + Features["ptx22"] = true;<br>
> + else if (Name == "ptx23")<br>
> + Features["ptx23"] = true;<br>
> + else if (Name == "sm10")<br>
> + Features["sm10"] = true;<br>
> + else if (Name == "sm11")<br>
> + Features["sm11"] = true;<br>
> + else if (Name == "sm12")<br>
> + Features["sm12"] = true;<br>
> + else if (Name == "sm13")<br>
> + Features["sm13"] = true;<br>
> + else if (Name == "sm20")<br>
> + Features["sm20"] = true;<br>
> + else if (Name == "sm21")<br>
> + Features["sm21"] = true;<br>
> + else if (Name == "sm22")<br>
> + Features["sm22"] = true;<br>
> + else if (Name == "sm23")<br>
> + Features["sm23"] = true;<br>
> + } else {<br>
> + if (Name == "double")<br>
> + Features["double"] = false;<br>
> + else if (Name == "no-fma")<br>
> + Features["no-fma"] = false;<br>
> + else if (Name == "compute10")<br>
> + Features["compute10"] = false;<br>
> + else if (Name == "compute11")<br>
> + Features["compute11"] = false;<br>
> + else if (Name == "compute12")<br>
> + Features["compute12"] = false;<br>
> + else if (Name == "compute13")<br>
> + Features["compute13"] = false;<br>
> + else if (Name == "compute20")<br>
> + Features["compute20"] = false;<br>
> + else if (Name == "ptx20")<br>
> + Features["ptx20"] = false;<br>
> + else if (Name == "ptx21")<br>
> + Features["ptx21"] = false;<br>
> + else if (Name == "ptx22")<br>
> + Features["ptx22"] = false;<br>
> + else if (Name == "ptx23")<br>
> + Features["ptx23"] = false;<br>
> + else if (Name == "sm10")<br>
> + Features["sm10"] = false;<br>
> + else if (Name == "sm11")<br>
> + Features["sm11"] = false;<br>
> + else if (Name == "sm12")<br>
> + Features["sm12"] = false;<br>
> + else if (Name == "sm13")<br>
> + Features["sm13"] = false;<br>
> + else if (Name == "sm20")<br>
> + Features["sm20"] = false;<br>
> + else if (Name == "sm21")<br>
> + Features["sm21"] = false;<br>
> + else if (Name == "sm22")<br>
> + Features["sm22"] = false;<br>
> + else if (Name == "sm23")<br>
> + Features["sm23"] = false;<br>
> + }<br>
<br>
Does the following code achieve the same?<br>
<br>
std::set<std::string> AvailableFeatures;<br>
AvailableFeatures.add("double");<br>
AvailableFeatures.add("no-fma");<br>
AvailableFeatures.add("compute10");<br>
[...]<br>
<br>
if (AvailableFeatures.count(Name))<br>
Features[Name] = Enabled;<br>
<br>
You may want to move the AvailableFeatures initialization in the<br>
constructure.<br>
<br>
Cheers<br>
Tobi<br></div></div></blockquote><div><br>I had basically the same remark :)<br><br>I would however suggest a sorted std::vector<llvm::StringRef> for AvailableFeatures. It provides a lesser memory footprint (about 16 bytes per entry) and offers basically the same complexity for the interface here: std::lower_bound(AvailableFeatures.begin(), AvailableFeatures.end(), Name) != AvailableFeatures.end(); though it'll probably be faster as well as it's got a better cache behavior.<br>
<br>Also, it could be possible to keep it close at hand by using a static local variable initialized with a simple function.<br></div></div></blockquote><div><br></div></div></div><div>Alright, I'll try to fix this by tomorrow.</div>
</div></blockquote><div><br></div></div></div><div>Fixed in r140320.</div><div class="im"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><div><font color="#888888"><br>-- Matthieu<br></font></div></div>
<br>_______________________________________________<br>
cfe-commits mailing list<div><br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
</div><div><a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
<br></div></blockquote></div><font color="#888888"><br><br clear="all"><div><br></div>-- <br><br><div>Thanks,</div><div><br></div><div>Justin Holewinski</div><br>
</font></blockquote></div></div><font color="#888888"><br><br clear="all"><div><br></div>-- <br><br><div>Thanks,</div><div><br></div><div>Justin Holewinski</div><br>
</font></blockquote></div>Thanks, it looks much neater.<br><br>I only have one remark: you placed a comment "must be in sorted order".<br><br>I can see two ways to ensure this:<br>> A call to `std::sort`, the vector is small anyway so it should not be too costly<br>
> An #ifndef NDEBUG (or equivalent) section to check that it is actually sorted in asserts builds<br><br>You might not find it's a big deal though, since the number of elements is small and can be encompass in a single glance.<br>
<br>-- Matthieu<br>