[llvm-commits] [llvm] r149215 - in /llvm/trunk: include/llvm/Bitcode/LLVMBitCodes.h lib/Bitcode/Reader/BitcodeReader.cpp lib/Bitcode/Writer/BitcodeWriter.cpp tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp

[Duncan Sands]

Hi Chris,

> Add bitcode reader and writer support for ConstantDataAggregate, which
> should be feature complete now.  Lets see if it works.

it does seem to work!

> --- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)
> +++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Sun Jan 29 18:51:16 2012
> @@ -1121,6 +1121,65 @@
>         V = ConstantArray::get(ATy, Elts);
>         break;
>       }
> +    case bitc::CST_CODE_DATA: {// DATA: [n x value]
> +      if (Record.empty())
> +        return Error("Invalid CST_DATA record");
> +
> +      Type *EltTy = cast<SequentialType>(CurTy)->getElementType();
> +      unsigned Size = Record.size();
> +
> +      if (EltTy->isIntegerTy(8)) {
> +        SmallVector<uint8_t, 16>  Elts(Record.begin(), Record.end());
> +        if (isa<VectorType>(CurTy))
> +          V = ConstantDataVector::get(Context, Elts);
> +        else
> +          V = ConstantDataArray::get(Context, Elts);

If CurTy is neither a VectorType nor an ArrayType, will the error be detected
later, or will we just crash?

> +      } else if (EltTy->isIntegerTy(16)) {


> --- llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp (original)
> +++ llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp Sun Jan 29 18:51:16 2012
> @@ -871,8 +871,55 @@
>           AbbrevToUse = CString6Abbrev;
>         else if (isCStr7)
>           AbbrevToUse = CString7Abbrev;
> -    } else if (isa<ConstantArray>(C) || isa<ConstantStruct>(V) ||
> -               isa<ConstantVector>(V)) {
> +    } else if (isa<ConstantDataSequential>(C)&&
> +               cast<ConstantDataSequential>(C)->isString()) {
> +      const ConstantDataSequential *Str = cast<ConstantDataSequential>(C);
...
> +    } else if (const ConstantDataSequential *CDS =
> +                  dyn_cast<ConstantDataSequential>(C)) {
...
> +    } else if (isa<ConstantArray>(C) || isa<ConstantStruct>(C) ||
> +               isa<ConstantVector>(C)) {

Both of these branches in essence do a dyn_cast<ConstantDataSequential>.  How
about doing:

   if (const ConstantDataSequential *CDS = dyn_cast<ConstantDataSequential>(C)) {
     if (CDS->isString()) {
     } else {
     }
   }

Perhaps you wanted to reduce indentation?

Ciao, Duncan.