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

[David Blaikie via llvm-commits]

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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160121/19172280/attachment-0001.html>