[llvm-commits] [llvm] r93339 - /llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp

[Victor Hernandez]

On Jan 13, 2010, at 1:46 PM, Devang Patel wrote:

> On Wed, Jan 13, 2010 at 1:19 PM, Victor Hernandez <vhernandez at apple.com> wrote:
>> 
>> On Jan 13, 2010, at 1:15 PM, Devang Patel wrote:
>> 
>> On Wed, Jan 13, 2010 at 1:07 PM, Victor Hernandez <vhernandez at apple.com>
>> wrote:
>> 
>> On Jan 13, 2010, at 1:05 PM, Devang Patel wrote:
>> 
>> On Wed, Jan 13, 2010 at 11:37 AM, Victor Hernandez <vhernandez at apple.com>
>> wrote:
>> 
>> 
>> +static void WriteFunctionLocalMetadata(const ValueEnumerator &VE,
>> 
>> +                                       BitstreamWriter &Stream) {
>> 
>> +  bool StartedMetadataBlock = false;
>> 
>> +  SmallVector<uint64_t, 64> Record;
>> 
>> +  ValueEnumerator::ValueList Vals = VE.getMDValues();
>> 
>> +  ValueEnumerator::ValueList::iterator it = Vals.begin();
>> 
>> +  ValueEnumerator::ValueList::iterator end = Vals.end();
>> 
>> +
>> 
>> +  while (it != end) {
>> 
>> +    if (const MDNode *N = dyn_cast<MDNode>((*it).first)) {
>> 
>> +      if (N->isFunctionLocal()) {
>> 
>> +        if (!StartedMetadataBlock) {
>> 
>> +          Stream.EnterSubblock(bitc::METADATA_BLOCK_ID, 3);
>> 
>> +          StartedMetadataBlock = true;
>> 
>> +        }
>> 
>> +        WriteMDNode(N, VE, Stream, Record);
>> 
>> +        // Remove function-local MD, since it is used outside of function.
>> 
>> how ?
>> 
>> Sorry that is a typo.  Meant to say "is not used".
>> 
>> but why remove from value list ?
>> 
>> Because the same ValueList (via VE.getMDValues()) is used during all of the
>> bitcode-writing.  Function-local metadata needs to exist in the ValueList
>> only during the writing of the function it is local to (from its lazy
>> enumeration during incorporateFunction until WriteFunctionLocalMetadata() is
>> called).  If it does not get removed here, then it will still be
>> in VE.getMDValues() when we go to write the next function.
>> 
>> 
>> 
>> +        it = Vals.erase(it);
>> 
>> +        end = Vals.end();
>> 
>> +        continue;
>> 
>> +      }
>> 
>> +    }
>> 
>> +    ++it;
>> 
>> +  }
>> 
>> +
>> 
>> +  if (StartedMetadataBlock)
>> 
>> +    Stream.ExitBlock();
>> 
>> +}
>> 
>> +
>> 
>>  static void WriteMetadataAttachment(const Function &F,
>> 
>>                                     const ValueEnumerator &VE,
>> 
>>                                     BitstreamWriter &Stream) {
>> 
>> @@ -1210,6 +1241,7 @@
>> 
>>   // Emit names for all the instructions etc.
>> 
>>   WriteValueSymbolTable(F.getValueSymbolTable(), VE, Stream);
>> 
>> +  WriteFunctionLocalMetadata(VE, Stream);
>> 
>> This does not check function so will it write function local metadata
>> 
>> from one function into another function's block ?
>> 
>> That check is done by the Verifier.  Do you think I need to do it here also?
>> 
>> Are you sure that incoming VE will not have function local metadata
>> for another function at this point?
>> 
>> Yes, that is guaranteed by removing function-local metadata as soon as they
>> get written.
>> 
> 
> ValueList is not manipulated like this in BitCode Writer. Erasing
> element one by one from ValueList may have performance impact.
> Instead, it is better to keep ValueList intact and check function in
> WriteFunctionLocalMetadata() and update isFunctionLocal() to return
> Function *.

Fixed in r93441.

> 
> -
> Devang