[llvm] r266513 - ValueMapper: Stop memoizing ConstantAsMetadata
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 15 20:39:46 PDT 2016
Author: dexonsmith
Date: Fri Apr 15 22:39:44 2016
New Revision: 266513
URL: http://llvm.org/viewvc/llvm-project?rev=266513&view=rev
Log:
ValueMapper: Stop memoizing ConstantAsMetadata
Stop memoizing ConstantAsMetadata in ValueMapper::mapMetadata. Now we
have to recompute it, but these metadata aren't particularly common, and
it restricts the lifetime of the Metadata map unnecessarily.
(The motivation is that I have a patch which uses a single Metadata map
for the lifetime of IRMover. Mehdi profiled r266446 with the patch
applied and we saw a pretty big speedup in lib/Linker.)
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=266513&r1=266512&r2=266513&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp Fri Apr 15 22:39:44 2016
@@ -505,14 +505,26 @@ bool MDNodeMapper::mapOperand(const Meta
return false;
if (Optional<Metadata *> MappedOp = M.mapSimpleMetadata(Op)) {
- assert((isa<MDString>(Op) || M.getVM().getMappedMD(Op)) &&
- "Expected result to be memoized");
+ if (auto *CMD = dyn_cast<ConstantAsMetadata>(Op))
+ assert((!*MappedOp || M.getVM().count(CMD->getValue()) ||
+ M.getVM().getMappedMD(Op)) &&
+ "Expected Value to be memoized");
+ else
+ assert((isa<MDString>(Op) || M.getVM().getMappedMD(Op)) &&
+ "Expected result to be memoized");
return *MappedOp != Op;
}
return push(*cast<MDNode>(Op)).HasChangedAddress;
}
+static ConstantAsMetadata *wrapConstantAsMetadata(const ConstantAsMetadata &CMD,
+ Value *MappedV) {
+ if (CMD.getValue() == MappedV)
+ return const_cast<ConstantAsMetadata *>(&CMD);
+ return MappedV ? ConstantAsMetadata::getConstant(MappedV) : nullptr;
+}
+
Optional<Metadata *> MDNodeMapper::getMappedOp(const Metadata *Op) const {
if (!Op)
return nullptr;
@@ -523,6 +535,9 @@ Optional<Metadata *> MDNodeMapper::getMa
if (isa<MDString>(Op))
return const_cast<Metadata *>(Op);
+ if (auto *CMD = dyn_cast<ConstantAsMetadata>(Op))
+ return wrapConstantAsMetadata(*CMD, M.getVM().lookup(CMD->getValue()));
+
return None;
}
@@ -720,6 +735,19 @@ Metadata *MDNodeMapper::map(const MDNode
return *getMappedOp(&FirstN);
}
+namespace {
+
+struct MapMetadataDisabler {
+ ValueToValueMapTy &VM;
+
+ MapMetadataDisabler(ValueToValueMapTy &VM) : VM(VM) {
+ VM.disableMapMetadata();
+ }
+ ~MapMetadataDisabler() { VM.enableMapMetadata(); }
+};
+
+} // end namespace
+
Optional<Metadata *> Mapper::mapSimpleMetadata(const Metadata *MD) {
// If the value already exists in the map, use it.
if (Optional<Metadata *> NewMD = getVM().getMappedMD(MD))
@@ -735,14 +763,13 @@ Optional<Metadata *> Mapper::mapSimpleMe
if (auto *CMD = dyn_cast<ConstantAsMetadata>(MD)) {
// Disallow recursion into metadata mapping through mapValue.
- getVM().disableMapMetadata();
- Value *MappedV = mapValue(CMD->getValue());
- getVM().enableMapMetadata();
-
- if (CMD->getValue() == MappedV)
- return mapToSelf(MD);
+ MapMetadataDisabler MMD(getVM());
- return mapToMetadata(MD, MappedV ? ValueAsMetadata::get(MappedV) : nullptr);
+ // Don't memoize ConstantAsMetadata. Instead of lasting until the
+ // LLVMContext is destroyed, they can be deleted when the GlobalValue they
+ // reference is destructed. These aren't super common, so the extra
+ // indirection isn't that expensive.
+ return wrapConstantAsMetadata(*CMD, mapValue(CMD->getValue()));
}
assert(isa<MDNode>(MD) && "Expected a metadata node");
Modified: llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp?rev=266513&r1=266512&r2=266513&view=diff
==============================================================================
--- llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp (original)
+++ llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp Fri Apr 15 22:39:44 2016
@@ -238,13 +238,14 @@ TEST(ValueMapperTest, mapMetadataConstan
auto *CAM = ConstantAsMetadata::get(F.get());
{
+ // ConstantAsMetadata shouldn't be memoized.
ValueToValueMapTy VM;
EXPECT_EQ(CAM, ValueMapper(VM).mapMetadata(*CAM));
- EXPECT_TRUE(VM.MD().count(CAM));
- VM.MD().erase(CAM);
+ EXPECT_FALSE(VM.MD().count(CAM));
EXPECT_EQ(CAM, ValueMapper(VM, RF_IgnoreMissingLocals).mapMetadata(*CAM));
- EXPECT_TRUE(VM.MD().count(CAM));
+ EXPECT_FALSE(VM.MD().count(CAM));
+ // But it should respect a mapping that gets seeded.
auto *N = MDTuple::get(C, None);
VM.MD()[CAM].reset(N);
EXPECT_EQ(N, ValueMapper(VM).mapMetadata(*CAM));
@@ -256,7 +257,7 @@ TEST(ValueMapperTest, mapMetadataConstan
ValueToValueMapTy VM;
VM[F.get()] = F2.get();
auto *F2MD = ValueMapper(VM).mapMetadata(*CAM);
- EXPECT_TRUE(VM.MD().count(CAM));
+ EXPECT_FALSE(VM.MD().count(CAM));
EXPECT_TRUE(F2MD);
EXPECT_EQ(F2.get(), cast<ConstantAsMetadata>(F2MD)->getValue());
}
More information about the llvm-commits
mailing list