[cfe-commits] r139789 - /cfe/trunk/lib/Basic/Targets.cpp
Matthieu Monrocq
matthieu.monrocq at gmail.com
Thu Sep 22 12:03:14 PDT 2011
2011/9/22 Justin Holewinski <justin.holewinski at gmail.com>
> On Thu, Sep 15, 2011 at 6:08 PM, Justin Holewinski <
> justin.holewinski at gmail.com> wrote:
>
>> On Thu, Sep 15, 2011 at 1:45 PM, Matthieu Monrocq <
>> matthieu.monrocq at gmail.com> wrote:
>>
>>>
>>> Date: Thu, 15 Sep 2011 15:34:14 +0100
>>>> From: Tobias Grosser <tobias at grosser.es>
>>>> Subject: Re: [cfe-commits] r139789 - /cfe/trunk/lib/Basic/Targets.cpp
>>>> To: Justin Holewinski <justin.holewinski at gmail.com>
>>>> Cc: cfe-commits at cs.uiuc.edu
>>>> Message-ID: <4E720CE6.9040001 at grosser.es>
>>>> Content-Type: text/plain; charset=ISO-8859-1; format=flowed
>>>>
>>>>
>>>> On 09/15/2011 01:13 PM, Justin Holewinski wrote:
>>>> > Author: jholewinski
>>>> > Date: Thu Sep 15 07:13:38 2011
>>>> > New Revision: 139789
>>>> >
>>>> > URL: http://llvm.org/viewvc/llvm-project?rev=139789&view=rev
>>>> > Log:
>>>> > PTX: Define target options
>>>> Hey Justin,
>>>>
>>>> this is nice, but you might shorten the code slightly.
>>>>
>>>> > Modified:
>>>> > cfe/trunk/lib/Basic/Targets.cpp
>>>> >
>>>> > Modified: cfe/trunk/lib/Basic/Targets.cpp
>>>> > URL:
>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets.cpp?rev=139789&r1=139788&r2=139789&view=diff
>>>> >
>>>> ==============================================================================
>>>> > --- cfe/trunk/lib/Basic/Targets.cpp (original)
>>>> > +++ cfe/trunk/lib/Basic/Targets.cpp Thu Sep 15 07:13:38 2011
>>>> > @@ -916,6 +916,10 @@
>>>> > // FIXME: implement
>>>> > return "typedef char* __builtin_va_list;";
>>>> > }
>>>> > +
>>>> > + virtual bool setFeatureEnabled(llvm::StringMap<bool> &Features,
>>>> > + const std::string&Name,
>>>> > + bool Enabled) const;
>>>> > };
>>>> >
>>>> > const Builtin::Info PTXTargetInfo::BuiltinInfo[] = {
>>>> > @@ -935,6 +939,91 @@
>>>> > NumNames = llvm::array_lengthof(GCCRegNames);
>>>> > }
>>>> >
>>>> > + bool PTXTargetInfo::setFeatureEnabled(llvm::StringMap<bool>
>>>> &Features,
>>>> > + const std::string&Name,
>>>> > + bool Enabled) const {
>>>> > + if (Enabled) {
>>>> > + if (Name == "double")
>>>> > + Features["double"] = true;
>>>> > + else if (Name == "no-fma")
>>>> > + Features["no-fma"] = true;
>>>> > + else if (Name == "compute10")
>>>> > + Features["compute10"] = true;
>>>> > + else if (Name == "compute11")
>>>> > + Features["compute11"] = true;
>>>> > + else if (Name == "compute12")
>>>> > + Features["compute12"] = true;
>>>> > + else if (Name == "compute13")
>>>> > + Features["compute13"] = true;
>>>> > + else if (Name == "compute20")
>>>> > + Features["compute20"] = true;
>>>> > + else if (Name == "ptx20")
>>>> > + Features["ptx20"] = true;
>>>> > + else if (Name == "ptx21")
>>>> > + Features["ptx21"] = true;
>>>> > + else if (Name == "ptx22")
>>>> > + Features["ptx22"] = true;
>>>> > + else if (Name == "ptx23")
>>>> > + Features["ptx23"] = true;
>>>> > + else if (Name == "sm10")
>>>> > + Features["sm10"] = true;
>>>> > + else if (Name == "sm11")
>>>> > + Features["sm11"] = true;
>>>> > + else if (Name == "sm12")
>>>> > + Features["sm12"] = true;
>>>> > + else if (Name == "sm13")
>>>> > + Features["sm13"] = true;
>>>> > + else if (Name == "sm20")
>>>> > + Features["sm20"] = true;
>>>> > + else if (Name == "sm21")
>>>> > + Features["sm21"] = true;
>>>> > + else if (Name == "sm22")
>>>> > + Features["sm22"] = true;
>>>> > + else if (Name == "sm23")
>>>> > + Features["sm23"] = true;
>>>> > + } else {
>>>> > + if (Name == "double")
>>>> > + Features["double"] = false;
>>>> > + else if (Name == "no-fma")
>>>> > + Features["no-fma"] = false;
>>>> > + else if (Name == "compute10")
>>>> > + Features["compute10"] = false;
>>>> > + else if (Name == "compute11")
>>>> > + Features["compute11"] = false;
>>>> > + else if (Name == "compute12")
>>>> > + Features["compute12"] = false;
>>>> > + else if (Name == "compute13")
>>>> > + Features["compute13"] = false;
>>>> > + else if (Name == "compute20")
>>>> > + Features["compute20"] = false;
>>>> > + else if (Name == "ptx20")
>>>> > + Features["ptx20"] = false;
>>>> > + else if (Name == "ptx21")
>>>> > + Features["ptx21"] = false;
>>>> > + else if (Name == "ptx22")
>>>> > + Features["ptx22"] = false;
>>>> > + else if (Name == "ptx23")
>>>> > + Features["ptx23"] = false;
>>>> > + else if (Name == "sm10")
>>>> > + Features["sm10"] = false;
>>>> > + else if (Name == "sm11")
>>>> > + Features["sm11"] = false;
>>>> > + else if (Name == "sm12")
>>>> > + Features["sm12"] = false;
>>>> > + else if (Name == "sm13")
>>>> > + Features["sm13"] = false;
>>>> > + else if (Name == "sm20")
>>>> > + Features["sm20"] = false;
>>>> > + else if (Name == "sm21")
>>>> > + Features["sm21"] = false;
>>>> > + else if (Name == "sm22")
>>>> > + Features["sm22"] = false;
>>>> > + else if (Name == "sm23")
>>>> > + Features["sm23"] = false;
>>>> > + }
>>>>
>>>> Does the following code achieve the same?
>>>>
>>>> std::set<std::string> AvailableFeatures;
>>>> AvailableFeatures.add("double");
>>>> AvailableFeatures.add("no-fma");
>>>> AvailableFeatures.add("compute10");
>>>> [...]
>>>>
>>>> if (AvailableFeatures.count(Name))
>>>> Features[Name] = Enabled;
>>>>
>>>> You may want to move the AvailableFeatures initialization in the
>>>> constructure.
>>>>
>>>> Cheers
>>>> Tobi
>>>>
>>>
>>> I had basically the same remark :)
>>>
>>> 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.
>>>
>>> Also, it could be possible to keep it close at hand by using a static
>>> local variable initialized with a simple function.
>>>
>>
>> Alright, I'll try to fix this by tomorrow.
>>
>
> Fixed in r140320.
>
>
>>
>>
>>>
>>> -- Matthieu
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>>
>>> cfe-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>>
>>
>>
>> --
>>
>> Thanks,
>>
>> Justin Holewinski
>>
>>
>
>
> --
>
> Thanks,
>
> Justin Holewinski
>
> Thanks, it looks much neater.
I only have one remark: you placed a comment "must be in sorted order".
I can see two ways to ensure this:
> A call to `std::sort`, the vector is small anyway so it should not be too
costly
> An #ifndef NDEBUG (or equivalent) section to check that it is actually
sorted in asserts builds
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.
-- Matthieu
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110922/05fa38d4/attachment.html>
More information about the cfe-commits
mailing list