[llvm-commits] [llvm] r44858 - in /llvm/trunk: include/llvm/Bitcode/LLVMBitCodes.h include/llvm/DerivedTypes.h include/llvm/GlobalVariable.h include/llvm/Instructions.h lib/AsmParser/LLLexer.cpp lib/AsmParser/llvmAsmParser.y lib/Bitcode/Reader/BitcodeReader.cpp lib/Bitcode/Writer/BitcodeWriter.cpp lib/VMCore/AsmWriter.cpp lib/VMCore/Constants.cpp lib/VMCore/Globals.cpp lib/VMCore/Instructions.cpp lib/VMCore/Type.cpp test/Assembler/2007-12-11-AddressSpaces.ll tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp

[Chris Lattner]

On Dec 11, 2007, at 9:16 PM, Christopher Lamb wrote:

>
> On Dec 11, 2007, at 4:12 PM, Chris Lattner wrote:
>
>> On Dec 11, 2007, at 12:59 AM, Christopher Lamb wrote:
>>
>>> =
>>> =
>>> ====================================================================
>>> ========
>>> --- llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp (original)
>>> +++ llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp Tue Dec 11
>>> 02:59:05 2007
>>> @@ -197,10 +197,14 @@
>>>        TypeVals.push_back(cast<IntegerType>(T)->getBitWidth());
>>>        break;
>>>      case Type::PointerTyID:
>>> +      const PointerType *PTy = cast<PointerType>(T);
>>> +      // POINTER: [pointee type] or [pointee type, address space]
>>>        Code = bitc::TYPE_CODE_POINTER;
>>> +      TypeVals.push_back(VE.getTypeID(PTy->getElementType()));
>>> +      if (unsigned AddressSpace = PTy->getAddressSpace())
>>> +        TypeVals.push_back(AddressSpace);
>>> +      else
>>> +        AbbrevToUse = PtrAbbrev;
>>>        break;
>>
>> This can be simplified.  In this code, I'd just unconditionally  
>> emit it:
>>
>>>      case Type::PointerTyID:
>>> +      const PointerType *PTy = cast<PointerType>(T);
>>> +      // POINTER: [pointee type] or [pointee type, address space]
>>>        Code = bitc::TYPE_CODE_POINTER;
>>> +      TypeVals.push_back(VE.getTypeID(PTy->getElementType()));
>>>        TypeVals.push_back(PTy->getAddressSpace());
>>>        AbbrevToUse = PtrAbbrev;
>>>        break;
>>
>> And change the abbreviation to match, which would apply only if the
>> addrspace is zero (and thus not encode it at all):
>>
>>    // Abbrev for TYPE_CODE_POINTER.
>>    BitCodeAbbrev *Abbv = new BitCodeAbbrev();
>>    Abbv->Add(BitCodeAbbrevOp(bitc::TYPE_CODE_POINTER));
>>    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed,
>>                              Log2_32_Ceil(VE.getTypes().size()+1)));
>>    Abbv->Add(BitCodeAbbrevOp(0));  // Addr space = 0.
>>    unsigned PtrAbbrev = Stream.EmitAbbrev(Abbv);
>>
>> The presence of the abbreviation means that any pointers with
>> addrspace 0 will not need per-instance space to represent this.
>
> This means moving the assert conditional into the if conditional and  
> removing the assert in BitstreamWriter.h. Sound OK?

Ah, right, sorry I misremembered.  In that case, make the PointerTyID  
handling code do something like:

   if (Addrspace == 0) AbbrevToUse = PtrAbbrev;

Thanks!

-Chris