[llvm] r258405 - [ThinLTO] Avoid unnecesary hash lookups during metadata linking (NFC)

[Teresa Johnson via llvm-commits]

Good idea. Done in r258407.
Thanks,
Teresa

On Thu, Jan 21, 2016 at 8:52 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Thu, Jan 21, 2016 at 8:46 AM, Teresa Johnson via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: tejohnson
>> Date: Thu Jan 21 10:46:40 2016
>> New Revision: 258405
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=258405&view=rev
>> Log:
>> [ThinLTO] Avoid unnecesary hash lookups during metadata linking (NFC)
>>
>> Replace sequences of count() followed by operator[] with either
>> find() or insert(), depending on the context.
>>
>> Modified:
>>     llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
>>     llvm/trunk/lib/Linker/IRMover.cpp
>>
>> Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=258405&r1=258404&r2=258405&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)
>> +++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Thu Jan 21 10:46:40
>> 2016
>> @@ -3081,9 +3081,11 @@ void BitcodeReader::saveMetadataList(
>>      if (!OnlyTempMD || (N && N->isTemporary())) {
>>        // Will call this after materializing each function, in order to
>>        // handle remapping of the function's instructions/metadata.
>> +      auto IterBool = MetadataToIDs.insert(std::make_pair(MD, ID));
>>        // See if we already have an entry in that case.
>> -      if (OnlyTempMD && MetadataToIDs.count(MD)) {
>> -        assert(MetadataToIDs[MD] == ID && "Inconsistent metadata value
>> id");
>> +      if (OnlyTempMD && !IterBool.second) {
>> +        assert(IterBool.first->second == ID &&
>> +               "Inconsistent metadata value id");
>>          continue;
>>        }
>>        if (N && N->isTemporary())
>> @@ -3091,7 +3093,6 @@ void BitcodeReader::saveMetadataList(
>>          // metadata while it is the key of a map. The flag will be set
>> back
>>          // to true when the saved metadata list is destroyed.
>>          N->setCanReplace(false);
>> -      MetadataToIDs[MD] = ID;
>>      }
>>    }
>>  }
>>
>> Modified: llvm/trunk/lib/Linker/IRMover.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Linker/IRMover.cpp?rev=258405&r1=258404&r2=258405&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Linker/IRMover.cpp (original)
>> +++ llvm/trunk/lib/Linker/IRMover.cpp Thu Jan 21 10:46:40 2016
>> @@ -659,17 +659,15 @@ Metadata *IRLinker::mapTemporaryMetadata
>>      return nullptr;
>>    // If this temporary metadata has a value id recorded during function
>>    // parsing, record that in the ValIDToTempMDMap if one was provided.
>> -  if (MetadataToIDs.count(MD)) {
>> -    unsigned Idx = MetadataToIDs[MD];
>> -    // Check if we created a temp MD when importing a different function
>> from
>> -    // this module. If so, reuse it the same temporary metadata,
>> otherwise
>> -    // add this temporary metadata to the map.
>> -    if (!ValIDToTempMDMap->count(Idx)) {
>> -      MDNode *Node = cast<MDNode>(MD);
>> -      assert(Node->isTemporary());
>> -      (*ValIDToTempMDMap)[Idx] = Node;
>> -    }
>> -    return (*ValIDToTempMDMap)[Idx];
>> +  auto I = MetadataToIDs.find(MD);
>> +  if (I != MetadataToIDs.end()) {
>>
>
> You could reduce indentation by switching this condition around and
> returning early, if you like:
>
>   if (I == end())
>     return;
>   unsigned Idx = ...;
>   ...
>
>
>> +    unsigned Idx = I->second;
>> +    MDNode *Node = cast<MDNode>(MD);
>> +    assert(Node->isTemporary());
>> +    // If we created a temp MD when importing a different function from
>> +    // this module, reuse the same temporary metadata.
>> +    auto IterBool = ValIDToTempMDMap->insert(std::make_pair(Idx, Node));
>> +    return IterBool.first->second;
>>    }
>>    return nullptr;
>>  }
>> @@ -686,16 +684,18 @@ void IRLinker::replaceTemporaryMetadata(
>>    // created during function importing was provided, and the source
>>    // metadata has a value id recorded during metadata parsing, replace
>>    // the temporary metadata with the final mapped metadata now.
>> -  if (MetadataToIDs.count(OrigMD)) {
>> -    unsigned Idx = MetadataToIDs[OrigMD];
>> +  auto I = MetadataToIDs.find(OrigMD);
>> +  if (I != MetadataToIDs.end()) {
>> +    unsigned Idx = I->second;
>> +    auto VI = ValIDToTempMDMap->find(Idx);
>>      // Nothing to do if we didn't need to create a temporary metadata
>> during
>>      // function importing.
>> -    if (!ValIDToTempMDMap->count(Idx))
>> +    if (VI == ValIDToTempMDMap->end())
>>        return;
>> -    MDNode *TempMD = (*ValIDToTempMDMap)[Idx];
>> +    MDNode *TempMD = VI->second;
>>      TempMD->replaceAllUsesWith(NewMD);
>>      MDNode::deleteTemporary(TempMD);
>> -    ValIDToTempMDMap->erase(Idx);
>> +    ValIDToTempMDMap->erase(VI);
>>    }
>>  }
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>


-- 
Teresa Johnson |  Software Engineer |  tejohnson at google.com |  408-460-2413
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160121/f6ceef08/attachment.html>