[llvm-commits] [llvm] r56704 - in /llvm/trunk: include/llvm/ lib/AsmParser/ lib/Bitcode/Reader/ lib/Transforms/IPO/ lib/Transforms/Scalar/ lib/VMCore/

[Duncan Sands]

Hi Devang,

> Return attributes use 0 as the index.
> Function attributes use ~0U as the index.

attributes are stored in order of the (unsigned) index.
Lookup traverses the list looking for the index.  So
using ~0U means looking up function attributes may be
particularly slow.  Since these are the attributes that
are probably looked up most often, I suggest either
making Index signed, so ~0U=-1 will come first, or
special casing ~0U lookup.

Ciao,

Duncan.

> +  //FIXME : In 3.0, stop accepting zext, sext and inreg as optional function 
> +  //attributes.

Does this mean that return value attributes are now placed next
to the return type and not at the end of the function declaration,
but you're accepting them in both positions for the moment?

> -  std::vector<AttrListPtr>().swap(Attributes);
> +  std::vector<AttrListPtr>().swap(MAttributes);

What does the M stand for in MAttributes?

> +      if (FnAttribute == Attribute::None && RetAttribute != Attribute::None) {
> +        if (RetAttribute & Attribute::NoUnwind) {
> +          FnAttribute = FnAttribute | Attribute::NoUnwind;
> +          RetAttribute = RetAttribute ^ Attribute::NoUnwind;
> +          useUpdatedAttrs = true;
> +        }
> +        if (RetAttribute & Attribute::NoReturn) {
> +          FnAttribute = FnAttribute | Attribute::NoReturn;
> +          RetAttribute = RetAttribute ^ Attribute::NoReturn;
> +          useUpdatedAttrs = true;
> +        }
> +        if (RetAttribute & Attribute::ReadOnly) {
> +          FnAttribute = FnAttribute | Attribute::ReadOnly;
> +          RetAttribute = RetAttribute ^ Attribute::ReadOnly;
> +          useUpdatedAttrs = true;
> +        }
> +        if (RetAttribute & Attribute::ReadNone) {
> +          FnAttribute = FnAttribute | Attribute::ReadNone;
> +          RetAttribute = RetAttribute ^ Attribute::ReadNone;
> +          useUpdatedAttrs = true;
> +        }
> +      }

Rather than hard-coding this list of attributes and repeating the
code, how about renaming

const Attributes ReturnOnly = NoReturn | NoUnwind | ReadNone | ReadOnly;

(in Attributes.h) to FunctionAttributes, and implement the above by doing
RetAttribute & FunctionAttributes to find the list of attributes to move,
or-ing that into FnAttribute etc.

> +  // Add any function attributes.
> +  if (Attributes attrs = PAL.getFnAttributes())
> +    AttributesVec.push_back(AttributeWithIndex::get(~0, attrs));

If Index becomes signed then this will need to be pushed at the front.

Ciao,

Duncan.