[llvm] r265827 - ValueMapper: Stop memoizing MDStrings

[Duncan P. N. Exon Smith via llvm-commits]

Author: dexonsmith
Date: Fri Apr  8 13:47:02 2016
New Revision: 265827

URL: http://llvm.org/viewvc/llvm-project?rev=265827&view=rev
Log:
ValueMapper: Stop memoizing MDStrings

Stop adding MDString to the Metadata section of the ValueMap in
MapMetadata.  It blows up the size of the map for no benefit, since we
can always return quickly anyway.

There is a potential follow-up that I don't think I'll push on right
away, but maybe someone else is interested:  stop checking for a
pre-mapped MDString, and move the `isa<MDString>()` checks in
Mapper::mapSimpleMetadata and MDNodeMapper::getMappedOp in front of the
`VM.getMappedMD()` calls.  While this would preclude explicitly
remapping MDStrings it would probably be a little faster.

Modified:
    llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
    llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp

Modified: llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp?rev=265827&r1=265826&r2=265827&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp Fri Apr  8 13:47:02 2016
@@ -434,7 +434,8 @@ bool MDNodeMapper::mapOperand(const Meta
     return false;
 
   if (Optional<Metadata *> MappedOp = M.mapSimpleMetadata(Op)) {
-    assert(M.VM.getMappedMD(Op) && "Expected result to be memoized");
+    assert((isa<MDString>(Op) || M.VM.getMappedMD(Op)) &&
+           "Expected result to be memoized");
     return *MappedOp != Op;
   }
 
@@ -448,6 +449,9 @@ Optional<Metadata *> MDNodeMapper::getMa
   if (Optional<Metadata *> MappedOp = M.VM.getMappedMD(Op))
     return *MappedOp;
 
+  if (isa<MDString>(Op))
+    return const_cast<Metadata *>(Op);
+
   return None;
 }
 
@@ -649,7 +653,7 @@ Optional<Metadata *> Mapper::mapSimpleMe
     return *NewMD;
 
   if (isa<MDString>(MD))
-    return mapToSelf(MD);
+    return const_cast<Metadata *>(MD);
 
   // This is a module-level metadata.  If nothing at the module level is
   // changing, use an identity mapping.

Modified: llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp?rev=265827&r1=265826&r2=265827&view=diff
==============================================================================
--- llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp (original)
+++ llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp Fri Apr  8 13:47:02 2016
@@ -140,6 +140,21 @@ TEST(ValueMapperTest, MapMetadataNullMap
   EXPECT_EQ(nullptr, MapValue(F.get(), VM, Flags));
 }
 
+TEST(ValueMapperTest, MapMetadataMDString) {
+  LLVMContext C;
+  auto *S1 = MDString::get(C, "S1");
+  ValueToValueMapTy VM;
+
+  // Make sure S1 maps to itself, but isn't memoized.
+  EXPECT_EQ(S1, MapMetadata(S1, VM));
+  EXPECT_EQ(None, VM.getMappedMD(S1));
+
+  // We still expect VM.MD() to be respected.
+  auto *S2 = MDString::get(C, "S2");
+  VM.MD()[S1].reset(S2);
+  EXPECT_EQ(S2, MapMetadata(S1, VM));
+}
+
 TEST(ValueMapperTest, MapMetadataConstantAsMetadata) {
   LLVMContext C;
   FunctionType *FTy =