[llvm-commits] [llvm] r47342 - in /llvm/trunk: include/llvm/ include/llvm/Support/ lib/AsmParser/ lib/Bitcode/Writer/ lib/Transforms/IPO/ lib/Transforms/Scalar/ lib/VMCore/ tools/llvm2cpp/

Dale Johannesen dalej at apple.com
Wed Feb 20 10:09:44 PST 2008


On Feb 19, 2008, at 8:04 PM, Chris Lattner wrote:
> First, the ParamAttrsWithIndex looks like it needs to grow the 'attrs'
> member to 32-bits.

I did; but I can't figure out if you misread the patch or meant  
something else, please look again.

> Second:
>
>> #include "llvm/BasicBlock.h"
>> #include "llvm/Argument.h"
>> #include "llvm/Support/Annotation.h"
>> +#include "llvm/ParameterAttributes.h"
>
> The problem with this (which was added in several places) is that this
> pulls in several headers just to get some enum values.
>
> llvm/ParameterAttributes.h contains two things: 1) a bunch of enums
> plus the ParamAttrsWithIndex struct and 2) the ParamAttrsVector +
> ParamAttrsList stuff.
>
> The former is just simple types, the later requires SmallVector and
> FoldingSet.
>
> Please split the second half of ParameterAttributes.h into a new
> ParamAttrsList.h file. ParamAttrsList.h should #include
> ParameterAttributes.h, so it serves as well as the old
> ParameterAttributes.h did.  However,   I'd like to make it so that
> other headers only include the former "light" header, and only .cpp
> files #include the full header.  Is this reasonable?

It seems natural to me to keep all the types that manipulate  
ParamAttrs together, and I really doubt there's a noticeable  
performance issue.  I guess I can do this, but why do you want to?

>> +const Attributes Alignment = 0xffff<<16; ///< Alignment of   
>> parameter (16 bits)
>> +                                    // 0 = unknown, else in clear   
>> (not log)
>
> 16 bits is a lot of bits for an alignment.  How about 4 bits stored in
> log form, where 0 -> alignment unspecified, and 1-15 mean "1 <<
> (n-1)"?  This gives us lots of bits for other crazy param attrs in the
> future.

There are the same number of free bits that there were before my  
change.  If I'd wanted to use a log representation I could have  
squeezed the alignment into the existing 16 bits; this way is easier  
to read and more efficient to access.  (And as a theoretical matter,  
alignments need not be powers of 2, although I don't know of a still- 
live target where that would be useful.)




More information about the llvm-commits mailing list