[cfe-commits] r139789 - /cfe/trunk/lib/Basic/Targets.cpp

[Matthieu Monrocq]

> 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.

-- Matthieu
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110915/2736ecb2/attachment.html>