[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

[Kostya Serebryany]

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


>  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?
We can probably come up with something like a proxy class to initialize the
Attribute constans, not sure if it's worth doing so.
What are your suggestions?

--kcc


> is to give up on type-safety and use a typedef for uint64_t, which I think
> is the easiest answer, if also the most dissatisfying.
>
> John.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120207/bd431dfc/attachment.html>