[cfe-commits] r155987 - in /cfe/trunk: lib/Sema/AttributeList.cpp utils/TableGen/ClangAttrEmitter.cpp

[Eli Friedman]

On Wed, May 2, 2012 at 12:14 PM, Matthieu Monrocq
<matthieu.monrocq at gmail.com> wrote:
>
>
> On Wed, May 2, 2012 at 8:22 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>>
>> On Wed, May 2, 2012 at 7:24 AM, Douglas Gregor <dgregor at apple.com> wrote:
>> > Author: dgregor
>> > Date: Wed May  2 09:24:30 2012
>> > New Revision: 155987
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=155987&view=rev
>> > Log:
>> > Replace the StringSwitch in AttributeList::getKind(const
>> > IdentifierInfo *) with a static StringMap, improving -fsyntax-only
>> > performance by 1% for the example in <rdar://problem/11004361>.
>> >
>> > Modified:
>> >    cfe/trunk/lib/Sema/AttributeList.cpp
>> >    cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
>> >
>> > Modified: cfe/trunk/lib/Sema/AttributeList.cpp
>> > URL:
>> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AttributeList.cpp?rev=155987&r1=155986&r2=155987&view=diff
>> >
>> > ==============================================================================
>> > --- cfe/trunk/lib/Sema/AttributeList.cpp (original)
>> > +++ cfe/trunk/lib/Sema/AttributeList.cpp Wed May  2 09:24:30 2012
>> > @@ -14,7 +14,7 @@
>> >  #include "clang/Sema/AttributeList.h"
>> >  #include "clang/AST/Expr.h"
>> >  #include "clang/Basic/IdentifierTable.h"
>> > -#include "llvm/ADT/StringSwitch.h"
>> > +#include "llvm/ADT/StringMap.h"
>> >  using namespace clang;
>> >
>> >  size_t AttributeList::allocated_size() const {
>> > @@ -97,6 +97,30 @@
>> >   return create(Name, TokLoc, 0, TokLoc, 0, TokLoc, &IArg, 1, 0);
>> >  }
>> >
>> > +
>> > +typedef llvm::StringMap<AttributeList::Kind> AttributeNameKindMap;
>> > +
>> > +static AttributeNameKindMap createAttributeNameKindMap(){
>> > +  AttributeNameKindMap Result;
>> > +#include "clang/Sema/AttrParsedAttrKinds.inc"
>> > +  Result["address_space"] = AttributeList::AT_address_space;
>> > +  Result["align"] = AttributeList::AT_aligned; // FIXME: should it be
>> > "aligned"?
>> > +  Result["base_check"] = AttributeList::AT_base_check;
>> > +  Result["bounded"] = AttributeList::IgnoredAttribute; // OpenBSD
>> > +  Result["__const"] = AttributeList::AT_const; // some GCC headers do
>> > contain this spelling
>> > +  Result["cf_returns_autoreleased"] =
>> > AttributeList::AT_cf_returns_autoreleased;
>> > +  Result["mode"] = AttributeList::AT_mode;
>> > +  Result["vec_type_hint"] = AttributeList::IgnoredAttribute;
>> > +  Result["ext_vector_type"] = AttributeList::AT_ext_vector_type;
>> > +  Result["neon_vector_type"] = AttributeList::AT_neon_vector_type;
>> > +  Result["neon_polyvector_type"] =
>> > AttributeList::AT_neon_polyvector_type;
>> > +  Result["opencl_image_access"] =
>> > AttributeList::AT_opencl_image_access;
>> > +  Result["objc_gc"] = AttributeList::AT_objc_gc;
>> > +  Result["objc_ownership"] = AttributeList::AT_objc_ownership;
>> > +  Result["vector_size"] = AttributeList::AT_vector_size;
>> > +  return Result;
>> > +}
>> > +
>> >  AttributeList::Kind AttributeList::getKind(const IdentifierInfo *Name)
>> > {
>> >   StringRef AttrName = Name->getName();
>> >
>> > @@ -105,22 +129,10 @@
>> >       AttrName.size() >= 4)
>> >     AttrName = AttrName.substr(2, AttrName.size() - 4);
>> >
>> > -  return llvm::StringSwitch<AttributeList::Kind>(AttrName)
>> > -    #include "clang/Sema/AttrParsedAttrKinds.inc"
>> > -    .Case("address_space", AT_address_space)
>> > -    .Case("align", AT_aligned) // FIXME - should it be "aligned"?
>> > -    .Case("base_check", AT_base_check)
>> > -    .Case("bounded", IgnoredAttribute)       // OpenBSD
>> > -    .Case("__const", AT_const) // some GCC headers do contain this
>> > spelling
>> > -    .Case("cf_returns_autoreleased", AT_cf_returns_autoreleased)
>> > -    .Case("mode", AT_mode)
>> > -    .Case("vec_type_hint", IgnoredAttribute)
>> > -    .Case("ext_vector_type", AT_ext_vector_type)
>> > -    .Case("neon_vector_type", AT_neon_vector_type)
>> > -    .Case("neon_polyvector_type", AT_neon_polyvector_type)
>> > -    .Case("opencl_image_access", AT_opencl_image_access)
>> > -    .Case("objc_gc", AT_objc_gc)
>> > -    .Case("objc_ownership", AT_objc_ownership)
>> > -    .Case("vector_size", AT_vector_size)
>> > -    .Default(UnknownAttribute);
>> > +  static AttributeNameKindMap Map = createAttributeNameKindMap();
>>
>> IIRC, it isn't safe to use local static variables with MSVC.
>>
>> -Eli
>>
> Hi Eli,
>
> I remember having issues with local static in headers and DLLs (VC++03
> managed to put one static per DLL... which is embarassing). It this what you
> are referring to or is it another issue (thread safety I'd guess) ?

I was thinking more along the lines of thread safety... but Doug
reverted this already, so we don't need to worry about it.

-Eli