From c1e407070844dd619893f12feae5d1217255a854 Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Thu, 7 Apr 2016 01:08:39 +0000 Subject: [PATCH] ValueMapper: Make LocalAsMetadata match function-local Values Start treating LocalAsMetadata similarly to function-local members of the Value hierarchy in MapValue and MapMetadata. - Don't memoize them. - Return nullptr if they are missing. This also cleans up ConstantAsMetadata to stop listening to the RF_IgnoreMissingLocals flag. llvm-svn: 265631 --- llvm/lib/Transforms/Utils/ValueMapper.cpp | 54 +++++++++----- .../unittests/Transforms/Utils/ValueMapperTest.cpp | 83 ++++++++++++++++++++++ 2 files changed, 121 insertions(+), 16 deletions(-) diff --git a/llvm/lib/Transforms/Utils/ValueMapper.cpp b/llvm/lib/Transforms/Utils/ValueMapper.cpp index 25c90b1..8b12cd4 100644 --- a/llvm/lib/Transforms/Utils/ValueMapper.cpp +++ b/llvm/lib/Transforms/Utils/ValueMapper.cpp @@ -86,6 +86,9 @@ public: /// (not an MDNode, or MDNode::isResolved() returns true). Metadata *mapMetadata(const Metadata *MD); + // Map LocalAsMetadata, which never gets memoized. + Metadata *mapLocalAsMetadata(const LocalAsMetadata &LAM); + private: Value *mapBlockAddress(const BlockAddress &BA); @@ -300,18 +303,27 @@ Value *Mapper::mapValue(const Value *V) { if (const auto *MDV = dyn_cast(V)) { const Metadata *MD = MDV->getMetadata(); + + // Locals shouldn't be memoized. Return nullptr if mapLocalAsMetadata() + // returns nullptr; otherwise bridge back to the Value hierarchy. + if (auto *LAM = dyn_cast(MD)) { + if (auto *MappedMD = mapLocalAsMetadata(*LAM)) { + if (LAM == MappedMD) + return const_cast(V); + return MetadataAsValue::get(V->getContext(), MappedMD); + } + return nullptr; + } + // If this is a module-level metadata and we know that nothing at the module // level is changing, then use an identity mapping. - if (!isa(MD) && (Flags & RF_NoModuleLevelChanges)) + if (Flags & RF_NoModuleLevelChanges) return VM[V] = const_cast(V); - // FIXME: be consistent with function-local values for LocalAsMetadata by - // returning nullptr when LocalAsMetadata is missing. Adding a mapping is - // expensive. + // Map the metadata and turn it into a value. auto *MappedMD = mapMetadata(MD); - if (MD == MappedMD || (!MappedMD && (Flags & RF_IgnoreMissingLocals))) + if (MD == MappedMD) return VM[V] = const_cast(V); - return VM[V] = MetadataAsValue::get(V->getContext(), MappedMD); } @@ -629,21 +641,16 @@ Optional Mapper::mapSimpleMetadata(const Metadata *MD) { if (isa(MD)) return mapToSelf(MD); - if (isa(MD)) + if (auto *CMD = dyn_cast(MD)) { if ((Flags & RF_NoModuleLevelChanges)) return mapToSelf(MD); - // FIXME: Assert that this is not LocalAsMetadata. It should be handled - // elsewhere. - if (const auto *VMD = dyn_cast(MD)) { // Disallow recursion into metadata mapping through mapValue. VM.disableMapMetadata(); - Value *MappedV = mapValue(VMD->getValue()); + Value *MappedV = mapValue(CMD->getValue()); VM.enableMapMetadata(); - // FIXME: Always use "ignore" behaviour. There should only be globals here. - if (VMD->getValue() == MappedV || - (!MappedV && (Flags & RF_IgnoreMissingLocals))) + if (CMD->getValue() == MappedV) return mapToSelf(MD); return mapToMetadata(MD, MappedV ? ValueAsMetadata::get(MappedV) : nullptr); @@ -665,9 +672,24 @@ Metadata *llvm::MapMetadata(const Metadata *MD, ValueToValueMapTy &VM, return Mapper(VM, Flags, TypeMapper, Materializer).mapMetadata(MD); } +Metadata *Mapper::mapLocalAsMetadata(const LocalAsMetadata &LAM) { + if (Optional NewMD = VM.getMappedMD(&LAM)) + return *NewMD; + + // Lookup the mapping for the value itself, and return the appropriate + // metadata. + if (Value *V = mapValue(LAM.getValue())) { + if (V == LAM.getValue()) + return const_cast(&LAM); + return ValueAsMetadata::get(V); + } + return nullptr; +} + Metadata *Mapper::mapMetadata(const Metadata *MD) { - // FIXME: First check for and deal with LocalAsMetadata, so that - // mapSimpleMetadata() doesn't need to deal with it. + if (auto *LAM = dyn_cast(MD)) + return mapLocalAsMetadata(*LAM); + if (Optional NewMD = mapSimpleMetadata(MD)) return *NewMD; diff --git a/llvm/unittests/Transforms/Utils/ValueMapperTest.cpp b/llvm/unittests/Transforms/Utils/ValueMapperTest.cpp index 865cb5d..56e407b 100644 --- a/llvm/unittests/Transforms/Utils/ValueMapperTest.cpp +++ b/llvm/unittests/Transforms/Utils/ValueMapperTest.cpp @@ -7,6 +7,7 @@ // //===----------------------------------------------------------------------===// +#include "llvm/IR/Function.h" #include "llvm/IR/LLVMContext.h" #include "llvm/IR/Metadata.h" #include "llvm/Transforms/Utils/ValueMapper.h" @@ -126,4 +127,86 @@ TEST(ValueMapperTest, MapMetadataSeededWithNull) { EXPECT_EQ(nullptr, MapMetadata(D, VM, RF_None)); } +TEST(ValueMapperTest, MapMetadataLocalAsMetadata) { + LLVMContext C; + FunctionType *FTy = + FunctionType::get(Type::getVoidTy(C), Type::getInt8Ty(C), false); + std::unique_ptr F( + Function::Create(FTy, GlobalValue::ExternalLinkage, "F")); + Argument &A = *F->arg_begin(); + + auto *LAM = LocalAsMetadata::get(&A); + ValueToValueMapTy VM; + EXPECT_EQ(nullptr, MapMetadata(LAM, VM)); + EXPECT_EQ(nullptr, MapMetadata(LAM, VM, RF_IgnoreMissingLocals)); + EXPECT_EQ(None, VM.getMappedMD(LAM)); + + VM.MD()[LAM].reset(LAM); + EXPECT_EQ(LAM, MapMetadata(LAM, VM)); + EXPECT_EQ(LAM, MapMetadata(LAM, VM, RF_IgnoreMissingLocals)); + + auto *N = MDNode::get(C, None); + VM.MD()[LAM].reset(N); + EXPECT_EQ(N, MapMetadata(LAM, VM)); + EXPECT_EQ(N, MapMetadata(LAM, VM, RF_IgnoreMissingLocals)); +} + +TEST(ValueMapperTest, MapMetadataConstantAsMetadata) { + LLVMContext C; + FunctionType *FTy = + FunctionType::get(Type::getVoidTy(C), Type::getInt8Ty(C), false); + std::unique_ptr F( + Function::Create(FTy, GlobalValue::ExternalLinkage, "F")); + + auto *CAM = ConstantAsMetadata::get(F.get()); + { + ValueToValueMapTy VM; + EXPECT_EQ(CAM, MapMetadata(CAM, VM)); + EXPECT_TRUE(VM.MD().count(CAM)); + VM.MD().erase(CAM); + EXPECT_EQ(CAM, MapMetadata(CAM, VM, RF_IgnoreMissingLocals)); + EXPECT_TRUE(VM.MD().count(CAM)); + + auto *N = MDNode::get(C, None); + VM.MD()[CAM].reset(N); + EXPECT_EQ(N, MapMetadata(CAM, VM)); + EXPECT_EQ(N, MapMetadata(CAM, VM, RF_IgnoreMissingLocals)); + } + + std::unique_ptr F2( + Function::Create(FTy, GlobalValue::ExternalLinkage, "F2")); + ValueToValueMapTy VM; + VM[F.get()] = F2.get(); + auto *F2MD = MapMetadata(CAM, VM); + EXPECT_TRUE(VM.MD().count(CAM)); + EXPECT_TRUE(F2MD); + EXPECT_EQ(F2.get(), cast(F2MD)->getValue()); +} + +TEST(ValueMapperTest, MapValueLocalAsMetadata) { + LLVMContext C; + FunctionType *FTy = + FunctionType::get(Type::getVoidTy(C), Type::getInt8Ty(C), false); + std::unique_ptr F( + Function::Create(FTy, GlobalValue::ExternalLinkage, "F")); + Argument &A = *F->arg_begin(); + + auto *LAM = LocalAsMetadata::get(&A); + auto *MAV = MetadataAsValue::get(C, LAM); + + ValueToValueMapTy VM; + EXPECT_EQ(nullptr, MapValue(MAV, VM)); + EXPECT_EQ(nullptr, MapValue(MAV, VM, RF_IgnoreMissingLocals)); + EXPECT_FALSE(VM.count(MAV)); + EXPECT_FALSE(VM.count(&A)); + + VM[MAV] = MAV; + EXPECT_EQ(MAV, MapValue(MAV, VM)); + EXPECT_EQ(MAV, MapValue(MAV, VM, RF_IgnoreMissingLocals)); + + VM[MAV] = &A; + EXPECT_EQ(&A, MapValue(MAV, VM)); + EXPECT_EQ(&A, MapValue(MAV, VM, RF_IgnoreMissingLocals)); +} + } // end namespace -- 2.7.4