ValueMapper: Make LocalAsMetadata match function-local Values
authorDuncan P. N. Exon Smith <dexonsmith@apple.com>
Thu, 7 Apr 2016 01:08:39 +0000 (01:08 +0000)
committerDuncan P. N. Exon Smith <dexonsmith@apple.com>
Thu, 7 Apr 2016 01:08:39 +0000 (01:08 +0000)
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
llvm/unittests/Transforms/Utils/ValueMapperTest.cpp

index 25c90b1..8b12cd4 100644 (file)
@@ -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<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 (!isa<LocalAsMetadata>(MD) && (Flags & RF_NoModuleLevelChanges))
+    if (Flags & RF_NoModuleLevelChanges)
       return VM[V] = const_cast<Value *>(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<Value *>(V);
-
     return VM[V] = MetadataAsValue::get(V->getContext(), MappedMD);
   }
 
@@ -629,21 +641,16 @@ Optional<Metadata *> Mapper::mapSimpleMetadata(const Metadata *MD) {
   if (isa<MDString>(MD))
     return mapToSelf(MD);
 
-  if (isa<ConstantAsMetadata>(MD))
+  if (auto *CMD = dyn_cast<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(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<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) {
-  // FIXME: First check for and deal with LocalAsMetadata, so that
-  // mapSimpleMetadata() doesn't need to deal with it.
+  if (auto *LAM = dyn_cast<LocalAsMetadata>(MD))
+    return mapLocalAsMetadata(*LAM);
+
   if (Optional<Metadata *> NewMD = mapSimpleMetadata(MD))
     return *NewMD;
 
index 865cb5d..56e407b 100644 (file)
@@ -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<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));
+}
+
 } // end namespace