[llvm-commits] CVS: llvm/lib/Target/TargetData.cpp

[Chris Lattner]

> Generalize TargetData strings, to support more interesting forms of  
> data.
> Patch by Scott Michel.

Scott,

> @@ -54,11 +48,10 @@
>    // Loop over each of the elements, placing them in memory...
>    for (unsigned i = 0, e = NumElements; i != e; ++i) {
>      const Type *Ty = ST->getElementType(i);
> -    unsigned char A;
>      unsigned TyAlign;
>      uint64_t TySize;
> -    getTypeInfoABI(Ty, &TD, TySize, A);
> -    TyAlign = ST->isPacked() ? 1 : A;
> +    TyAlign = (unsigned) TD.getABITypeAlignment(Ty);
> +    TySize = (unsigned) TD.getTypeSize(Ty);

Why are you casting typesize to unsigned here?  This breaks support  
for structs that are are bigger than 4G on a 64-bit system.

> +TargetAlignElem
> +TargetAlignElem::get(AlignTypeEnum align_type, unsigned char  
> abi_align,
> +                     unsigned char pref_align, short bit_width)
> +{

Please use K&R style braces ( '{' on same line) for consistency with  
the rest of this file.


> +  // FIXME: Is this still necessary?
> +  const TargetAlignElem &long_align = getAlignment(INTEGER_ALIGN,  
> 64);
> +  if (long_align.ABIAlign == 0)
> +    setAlignment(INTEGER_ALIGN, PointerMemSize, PointerMemSize, 64);

Please decide if this is needed :)

> +struct hyphen_delimited :
> +  public std::iterator<std::output_iterator_tag, void, void, void,  
> void>

This is very cute, but also very obtuse.  :)  Please write it as a  
simple loop for maintainability.

>  std::string TargetData::getStringRepresentation() const {
>    std::stringstream repr;

Also, you should be able to eliminate the stringstream here.  Just  
use 'ustostr' from llvm/ADT/StringExtras.h

>
> -uint64_t TargetData::getTypeSize(const Type *Ty) const {
> -  uint64_t Size;
> -  unsigned char Align;
> -  getTypeInfoABI(Ty, this, Size, Align);
> -  return Size;
> +  const TargetAlignElem &elem = getAlignment((AlignTypeEnum)  
> AlignType,
> +                                             getTypeSize(Ty) * 8);
> +  if (validAlignment(elem))
> +    return (abi_or_pref ? elem.ABIAlign : elem.PrefAlign);
> +  else {
> +    cerr << "TargetData::getAlignment: align type " << AlignType
> +         << " size " << getTypeSize(Ty) << " not found in  
> Alignments.\n";

Please turn this into an assert message.

Overall, very nice work Scott!

-Chris