[llvm] r265637 - Revert "ValueMapper: Make LocalAsMetadata match function-local Values"

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 6 19:10:51 PDT 2016


Author: dexonsmith
Date: Wed Apr  6 21:10:50 2016
New Revision: 265637

URL: http://llvm.org/viewvc/llvm-project?rev=265637&view=rev
Log:
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.

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=265637&r1=265636&r2=265637&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/ValueMapper.cpp Wed Apr  6 21:10:50 2016
@@ -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<MetadataAsValue>(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<LocalAsMetadata>(MD)) {
-      if (auto *MappedMD = mapLocalAsMetadata(*LAM)) {
-        if (LAM == MappedMD)
-          return const_cast<Value *>(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<LocalAsMetadata>(MD) && (Flags & RF_NoModuleLevelChanges))
       return VM[V] = const_cast<Value *>(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<Value *>(V);
+
     return VM[V] = MetadataAsValue::get(V->getContext(), MappedMD);
   }
 
@@ -635,16 +623,21 @@ Optional<Metadata *> Mapper::mapSimpleMe
   if (isa<MDString>(MD))
     return mapToSelf(MD);
 
-  if (auto *CMD = dyn_cast<ConstantAsMetadata>(MD)) {
+  if (isa<ConstantAsMetadata>(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<ValueAsMetadata>(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 Metada
   return Mapper(VM, Flags, TypeMapper, Materializer).mapMetadata(MD);
 }
 
-Metadata *Mapper::mapLocalAsMetadata(const LocalAsMetadata &LAM) {
-  if (Optional<Metadata *> 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<LocalAsMetadata *>(&LAM);
-    return ValueAsMetadata::get(V);
-  }
-  return nullptr;
-}
-
 Metadata *Mapper::mapMetadata(const Metadata *MD) {
-  if (auto *LAM = dyn_cast<LocalAsMetadata>(MD))
-    return mapLocalAsMetadata(*LAM);
-
+  // FIXME: First check for and deal with LocalAsMetadata, so that
+  // mapSimpleMetadata() doesn't need to deal with it.
   if (Optional<Metadata *> NewMD = mapSimpleMetadata(MD))
     return *NewMD;
 

Modified: llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp?rev=265637&r1=265636&r2=265637&view=diff
==============================================================================
--- llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp (original)
+++ llvm/trunk/unittests/Transforms/Utils/ValueMapperTest.cpp Wed Apr  6 21:10:50 2016
@@ -127,88 +127,6 @@ TEST(ValueMapperTest, MapMetadataSeededW
   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<Function> 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<Function> 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<Function> 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<ConstantAsMetadata>(F2MD)->getValue());
-}
-
-TEST(ValueMapperTest, MapValueLocalAsMetadata) {
-  LLVMContext C;
-  FunctionType *FTy =
-      FunctionType::get(Type::getVoidTy(C), Type::getInt8Ty(C), false);
-  std::unique_ptr<Function> 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 =




More information about the llvm-commits mailing list