[llvm-commits] [llvm] r148553 - in /llvm/trunk: docs/LangRef.html include/llvm-c/Core.h include/llvm/Attributes.h lib/AsmParser/LLLexer.cpp lib/AsmParser/LLParser.cpp lib/AsmParser/LLParser.h lib/AsmParser/LLToken.h lib/Bitcode/Reader/BitcodeRead

[John McCall]

On Feb 7, 2012, at 12:12 PM, Kostya Serebryany wrote:
> On Tue, Feb 7, 2012 at 11:42 AM, John McCall <rjmccall at apple.com> wrote:
> On Jan 20, 2012, at 9:56 AM, Kostya Serebryany wrote:
> > Author: kcc
> > Date: Fri Jan 20 11:56:17 2012
> > New Revision: 148553
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=148553&view=rev
> > Log:
> > Extend Attributes to 64 bits
> >
> > Problem: LLVM needs more function attributes than currently available (32 bits).
> > One such proposed attribute is "address_safety", which shows that a function is being checked for address safety (by AddressSanitizer, SAFECode, etc).
> >
> > Solution:
> > - extend the Attributes from 32 bits to 64-bits
> > - wrap the object into a class so that unsigned is never erroneously used instead
> > - change "unsigned" to "Attributes" throughout the code, including one place in clang.
> > - the class has no "operator uint64 ()", but it has "uint64_t Raw() " to support packing/unpacking.
> > - the class has "safe operator bool()" to support the common idiom:  if (Attributes attr = getAttrs()) useAttrs(attr);
> > - The CTOR from uint64_t is marked explicit, so I had to add a few explicit CTOR calls
> > - Add the new attribute "address_safety". Doing it in the same commit to check that attributes beyond first 32 bits actually work.
> > - Some of the functions from the Attribute namespace are worth moving inside the class, but I'd prefer to have it as a separate commit.
> >
> > Tested:
> > "make check" on Linux (32-bit and 64-bit) and Mac (10.6)
> > built/run spec CPU 2006 on Linux with clang -O2.
> 
> Using a class here is introducing global initializers into every translation unit that includes llvm/Attributes.h.  Those initializers are optimized out at -O1 (by clang, at least), but it's still bad practice.
> 
> Why?
> This code looks clean and effective (with >=O1). 

The main problem is that it triggers warnings about global constructors.

>  If you remove all the constructors from the class, you can use list-initialization instead, which (in this case) is essentially guaranteed to not require a global initializer;  the only other alternative (given MSVC's limitations)
> 
> What are these limitations? 

I believe the underlying type of an enum on MSVC is always 'int', regardless of its enumerators.  In any case, it's always 32 bits.  Just for the record, no, that's not standards-compliant.

> We can probably come up with something like a proxy class to initialize the Attribute constans, not sure if it's worth doing so.

Oh, that should work well enough;  just use a proxy, list-initializable class that implicitly converts to your class type.

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120207/3d2fc855/attachment.html>