From: Duncan P. N. Exon Smith Date: Thu, 7 Apr 2016 02:10:50 +0000 (+0000) Subject: Revert "ValueMapper: Make LocalAsMetadata match function-local Values" X-Git-Tag: llvmorg-3.9.0-rc1~9776 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=45601e867d6990b50b1ad54c8db7bbac0950684d;p=platform%2Fupstream%2Fllvm.git Revert "ValueMapper: Make LocalAsMetadata match function-local Values" This reverts commit r265631, since it caused bot failures: http://lab.llvm.org:8011/builders/clang-ppc64be-linux/builds/3256 http://lab.llvm.org:8011/builders/clang-cmake-aarch64-42vma/builds/7272 Looks like something is depending on the old behaviour. I'll try to track it down and recommit. llvm-svn: 265637 --- diff --git a/llvm/lib/Transforms/Utils/ValueMapper.cpp b/llvm/lib/Transforms/Utils/ValueMapper.cpp index 5a92b4c..59870f8 100644 --- a/llvm/lib/Transforms/Utils/ValueMapper.cpp +++ b/llvm/lib/Transforms/Utils/ValueMapper.cpp @@ -86,9 +86,6 @@ 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); @@ -297,27 +294,18 @@ 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 (Flags & RF_NoModuleLevelChanges) + if (!isa(MD) && (Flags & RF_NoModuleLevelChanges)) return VM[V] = const_cast(V); - // Map the metadata and turn it into a value. + // FIXME: be consistent with function-local values for LocalAsMetadata by + // returning nullptr when LocalAsMetadata is missing. Adding a mapping is + // expensive. auto *MappedMD = mapMetadata(MD); - if (MD == MappedMD) + if (MD == MappedMD || (!MappedMD && (Flags & RF_IgnoreMissingLocals))) return VM[V] = const_cast(V); + return VM[V] = MetadataAsValue::get(V->getContext(), MappedMD); } @@ -635,16 +623,21 @@ Optional Mapper::mapSimpleMetadata(const Metadata *MD) { if (isa(MD)) return mapToSelf(MD); - if (auto *CMD = dyn_cast(MD)) { + if (isa(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(CMD->getValue()); + Value *MappedV = mapValue(VMD->getValue()); VM.enableMapMetadata(); - if (CMD->getValue() == MappedV) + // FIXME: Always use "ignore" behaviour. There should only be globals here. + if (VMD->getValue() == MappedV || + (!MappedV && (Flags & RF_IgnoreMissingLocals))) return mapToSelf(MD); return mapToMetadata(MD, MappedV ? ValueAsMetadata::get(MappedV) : nullptr); @@ -666,24 +659,9 @@ 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) { - if (auto *LAM = dyn_cast(MD)) - return mapLocalAsMetadata(*LAM); - + // FIXME: First check for and deal with LocalAsMetadata, so that + // mapSimpleMetadata() doesn't need to deal with it. if (Optional NewMD = mapSimpleMetadata(MD)) return *NewMD; diff --git a/llvm/unittests/Transforms/Utils/ValueMapperTest.cpp b/llvm/unittests/Transforms/Utils/ValueMapperTest.cpp index b9cca22..222f2a2 100644 --- a/llvm/unittests/Transforms/Utils/ValueMapperTest.cpp +++ b/llvm/unittests/Transforms/Utils/ValueMapperTest.cpp @@ -127,88 +127,6 @@ 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)); -} - TEST(ValueMapperTest, MapMetadataNullMapGlobalWithIgnoreMissingLocals) { LLVMContext C; FunctionType *FTy =