[llvm] r232275 - IR: Make Metadata::print() reliable and useful

Duncan P. N. Exon Smith dexonsmith at apple.com
Sat Mar 14 13:19:36 PDT 2015


Author: dexonsmith
Date: Sat Mar 14 15:19:36 2015
New Revision: 232275

URL: http://llvm.org/viewvc/llvm-project?rev=232275&view=rev
Log:
IR: Make Metadata::print() reliable and useful

Replumb the `AsmWriter` so that `Metadata::print()` is generally useful.
(Similarly change `Metadata::printAsOperand()`.)

- `SlotTracker` now has a mode where all metadata will be correctly
  numbered when initializing a `Module`.  Normally, `Metadata` only
  referenced from within `Function`s gets numbered when the `Function`
  is incorporated.
- `Metadata::print()` and `Metadata::printAsOperand()` (and
  `Metadata::dump()`) now take an optional `Module` argument.  When
  provided, `SlotTracker` is initialized with the new mode, and the
  numbering will be complete and consistent for all calls to `print()`.
- `Value::print()` uses the new `SlotTracker` mode when printing
  intrinsics with `MDNode` operands, `MetadataAsValue` operands, or the
  bodies of functions.  Thus, metadata numbering will be consistent
  between calls to `Metadata::print()` and `Value::print()`.
- `Metadata::print()` (and `Metadata::dump()`) now print the full
  definition of `MDNode`s:

    !5 = !{!6, !"abc", !7}

  This matches behaviour for `Value::print()`, which includes the name
  of instructions.
- Updated call sites in `Verifier` to call `print()` instead of
  `printAsOperand()`.

All this, so that `Verifier` can print out useful failure messages that
involve `Metadata` for PR22777.

Note that `Metadata::printAsOperand()` previously took an optional
`bool` and `Module` operand.  The former was cargo-culted from
`Value::printAsOperand()` and wasn't doing anything useful.  The latter
didn't give consistent results (without the new `SlotTracker` mode).

Modified:
    llvm/trunk/include/llvm/IR/Metadata.h
    llvm/trunk/lib/IR/AsmWriter.cpp
    llvm/trunk/lib/IR/Verifier.cpp
    llvm/trunk/unittests/IR/MetadataTest.cpp

Modified: llvm/trunk/include/llvm/IR/Metadata.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Metadata.h?rev=232275&r1=232274&r2=232275&view=diff
==============================================================================
--- llvm/trunk/include/llvm/IR/Metadata.h (original)
+++ llvm/trunk/include/llvm/IR/Metadata.h Sat Mar 14 15:19:36 2015
@@ -103,10 +103,26 @@ public:
   unsigned getMetadataID() const { return SubclassID; }
 
   /// \brief User-friendly dump.
-  void dump() const;
-  void print(raw_ostream &OS) const;
-  void printAsOperand(raw_ostream &OS, bool PrintType = true,
-                      const Module *M = nullptr) const;
+  ///
+  /// If \c M is provided, metadata nodes will be numbered canonically;
+  /// otherwise, pointer addresses are substituted.
+  void dump(const Module *M = nullptr) const;
+
+  /// \brief Print.
+  ///
+  /// Prints definition of \c this.
+  ///
+  /// If \c M is provided, metadata nodes will be numbered canonically;
+  /// otherwise, pointer addresses are substituted.
+  void print(raw_ostream &OS, const Module *M = nullptr) const;
+
+  /// \brief Print as operand.
+  ///
+  /// Prints reference of \c this.
+  ///
+  /// If \c M is provided, metadata nodes will be numbered canonically;
+  /// otherwise, pointer addresses are substituted.
+  void printAsOperand(raw_ostream &OS, const Module *M = nullptr) const;
 };
 
 #define HANDLE_METADATA(CLASS) class CLASS;

Modified: llvm/trunk/lib/IR/AsmWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/AsmWriter.cpp?rev=232275&r1=232274&r2=232275&view=diff
==============================================================================
--- llvm/trunk/lib/IR/AsmWriter.cpp (original)
+++ llvm/trunk/lib/IR/AsmWriter.cpp Sat Mar 14 15:19:36 2015
@@ -275,6 +275,15 @@ static const Module *getModuleFromVal(co
 
   if (const GlobalValue *GV = dyn_cast<GlobalValue>(V))
     return GV->getParent();
+
+  if (const auto *MAV = dyn_cast<MetadataAsValue>(V)) {
+    for (const User *U : MAV->users())
+      if (isa<Instruction>(U))
+        if (const Module *M = getModuleFromVal(U))
+          return M;
+    return nullptr;
+  }
+
   return nullptr;
 }
 
@@ -525,6 +534,7 @@ private:
   /// TheFunction - The function for which we are holding slot numbers.
   const Function* TheFunction;
   bool FunctionProcessed;
+  bool ShouldInitializeAllMetadata;
 
   /// mMap - The slot map for the module level data.
   ValueMap mMap;
@@ -542,10 +552,20 @@ private:
   DenseMap<AttributeSet, unsigned> asMap;
   unsigned asNext;
 public:
-  /// Construct from a module
-  explicit SlotTracker(const Module *M);
+  /// Construct from a module.
+  ///
+  /// If \c ShouldInitializeAllMetadata, initializes all metadata in all
+  /// functions, giving correct numbering for metadata referenced only from
+  /// within a function (even if no functions have been initialized).
+  explicit SlotTracker(const Module *M,
+                       bool ShouldInitializeAllMetadata = false);
   /// Construct from a function, starting out in incorp state.
-  explicit SlotTracker(const Function *F);
+  ///
+  /// If \c ShouldInitializeAllMetadata, initializes all metadata in all
+  /// functions, giving correct numbering for metadata referenced only from
+  /// within a function (even if no functions have been initialized).
+  explicit SlotTracker(const Function *F,
+                       bool ShouldInitializeAllMetadata = false);
 
   /// Return the slot number of the specified value in it's type
   /// plane.  If something is not in the SlotTracker, return -1.
@@ -606,6 +626,9 @@ private:
   /// Add all of the functions arguments, basic blocks, and instructions.
   void processFunction();
 
+  /// Add all of the metadata from a function.
+  void processFunctionMetadata(const Function &F);
+
   /// Add all of the metadata from an instruction.
   void processInstructionMetadata(const Instruction &I);
 
@@ -648,15 +671,18 @@ static SlotTracker *createSlotTracker(co
 
 // Module level constructor. Causes the contents of the Module (sans functions)
 // to be added to the slot table.
-SlotTracker::SlotTracker(const Module *M)
-    : TheModule(M), TheFunction(nullptr), FunctionProcessed(false), mNext(0),
+SlotTracker::SlotTracker(const Module *M, bool ShouldInitializeAllMetadata)
+    : TheModule(M), TheFunction(nullptr), FunctionProcessed(false),
+      ShouldInitializeAllMetadata(ShouldInitializeAllMetadata), mNext(0),
       fNext(0), mdnNext(0), asNext(0) {}
 
 // Function level constructor. Causes the contents of the Module and the one
 // function provided to be added to the slot table.
-SlotTracker::SlotTracker(const Function *F)
+SlotTracker::SlotTracker(const Function *F, bool ShouldInitializeAllMetadata)
     : TheModule(F ? F->getParent() : nullptr), TheFunction(F),
-      FunctionProcessed(false), mNext(0), fNext(0), mdnNext(0), asNext(0) {}
+      FunctionProcessed(false),
+      ShouldInitializeAllMetadata(ShouldInitializeAllMetadata), mNext(0),
+      fNext(0), mdnNext(0), asNext(0) {}
 
 inline void SlotTracker::initialize() {
   if (TheModule) {
@@ -695,6 +721,9 @@ void SlotTracker::processModule() {
       // Add all the unnamed functions to the table.
       CreateModuleSlot(I);
 
+    if (ShouldInitializeAllMetadata)
+      processFunctionMetadata(*I);
+
     // Add all the function attributes to the table.
     // FIXME: Add attributes of other objects?
     AttributeSet FnAttrs = I->getAttributes().getFnAttributes();
@@ -750,6 +779,12 @@ void SlotTracker::processFunction() {
   ST_DEBUG("end processFunction!\n");
 }
 
+void SlotTracker::processFunctionMetadata(const Function &F) {
+  for (auto &BB : F)
+    for (auto &I : BB)
+      processInstructionMetadata(I);
+}
+
 void SlotTracker::processInstructionMetadata(const Instruction &I) {
   // Process metadata used directly by intrinsics.
   if (const CallInst *CI = dyn_cast<CallInst>(&I))
@@ -3134,11 +3169,24 @@ void Type::print(raw_ostream &OS) const
     }
 }
 
+static bool isReferencingMDNode(const Instruction &I) {
+  if (const auto *CI = dyn_cast<CallInst>(&I))
+    if (Function *F = CI->getCalledFunction())
+      if (F->isIntrinsic())
+        for (auto &Op : I.operands())
+          if (auto *V = dyn_cast_or_null<MetadataAsValue>(Op))
+            if (isa<MDNode>(V->getMetadata()))
+              return true;
+  return false;
+}
+
 void Value::print(raw_ostream &ROS) const {
   formatted_raw_ostream OS(ROS);
   if (const Instruction *I = dyn_cast<Instruction>(this)) {
     const Function *F = I->getParent() ? I->getParent()->getParent() : nullptr;
-    SlotTracker SlotTable(F);
+    SlotTracker SlotTable(
+        F,
+        /* ShouldInitializeAllMetadata */ isReferencingMDNode(*I));
     AssemblyWriter W(OS, SlotTable, getModuleFromVal(I), nullptr);
     W.printInstruction(*I);
   } else if (const BasicBlock *BB = dyn_cast<BasicBlock>(this)) {
@@ -3146,7 +3194,8 @@ void Value::print(raw_ostream &ROS) cons
     AssemblyWriter W(OS, SlotTable, getModuleFromVal(BB), nullptr);
     W.printBasicBlock(BB);
   } else if (const GlobalValue *GV = dyn_cast<GlobalValue>(this)) {
-    SlotTracker SlotTable(GV->getParent());
+    SlotTracker SlotTable(GV->getParent(),
+                          /* ShouldInitializeAllMetadata */ isa<Function>(GV));
     AssemblyWriter W(OS, SlotTable, GV->getParent(), nullptr);
     if (const GlobalVariable *V = dyn_cast<GlobalVariable>(GV))
       W.printGlobal(V);
@@ -3155,7 +3204,7 @@ void Value::print(raw_ostream &ROS) cons
     else
       W.printAlias(cast<GlobalAlias>(GV));
   } else if (const MetadataAsValue *V = dyn_cast<MetadataAsValue>(this)) {
-    V->getMetadata()->print(ROS);
+    V->getMetadata()->print(ROS, getModuleFromVal(V));
   } else if (const Constant *C = dyn_cast<Constant>(this)) {
     TypePrinting TypePrinter;
     TypePrinter.print(C->getType(), OS);
@@ -3171,8 +3220,9 @@ void Value::print(raw_ostream &ROS) cons
 void Value::printAsOperand(raw_ostream &O, bool PrintType, const Module *M) const {
   // Fast path: Don't construct and populate a TypePrinting object if we
   // won't be needing any types printed.
-  if (!PrintType && ((!isa<Constant>(this) && !isa<MetadataAsValue>(this)) ||
-                     hasName() || isa<GlobalValue>(this))) {
+  bool IsMetadata = isa<MetadataAsValue>(this);
+  if (!PrintType && ((!isa<Constant>(this) && !IsMetadata) || hasName() ||
+                     isa<GlobalValue>(this))) {
     WriteAsOperandInternal(O, this, nullptr, nullptr, M);
     return;
   }
@@ -3188,33 +3238,35 @@ void Value::printAsOperand(raw_ostream &
     O << ' ';
   }
 
-  WriteAsOperandInternal(O, this, &TypePrinter, nullptr, M);
+  SlotTracker Machine(M, /* ShouldInitializeAllMetadata */ IsMetadata);
+  WriteAsOperandInternal(O, this, &TypePrinter, &Machine, M);
 }
 
-void Metadata::print(raw_ostream &ROS) const {
+static void printMetadataImpl(raw_ostream &ROS, const Metadata &MD,
+                              const Module *M, bool OnlyAsOperand) {
   formatted_raw_ostream OS(ROS);
-  if (auto *N = dyn_cast<MDNode>(this)) {
-    SlotTracker SlotTable(static_cast<Function *>(nullptr));
-    AssemblyWriter W(OS, SlotTable, nullptr, nullptr);
-    W.printMDNodeBody(N);
 
+  auto *N = dyn_cast<MDNode>(&MD);
+  TypePrinting TypePrinter;
+  SlotTracker Machine(M, /* ShouldInitializeAllMetadata */ N);
+  if (M)
+    TypePrinter.incorporateTypes(*M);
+
+  WriteAsOperandInternal(OS, &MD, &TypePrinter, &Machine, M,
+                         /* FromValue */ true);
+  if (OnlyAsOperand || !N)
     return;
-  }
-  printAsOperand(OS);
+
+  OS << " = ";
+  WriteMDNodeBodyInternal(OS, N, &TypePrinter, &Machine, M);
 }
 
-void Metadata::printAsOperand(raw_ostream &ROS, bool PrintType,
-                              const Module *M) const {
-  formatted_raw_ostream OS(ROS);
+void Metadata::printAsOperand(raw_ostream &OS, const Module *M) const {
+  printMetadataImpl(OS, *this, M, /* OnlyAsOperand */ true);
+}
 
-  std::unique_ptr<TypePrinting> TypePrinter;
-  if (PrintType) {
-    TypePrinter.reset(new TypePrinting);
-    if (M)
-      TypePrinter->incorporateTypes(*M);
-  }
-  WriteAsOperandInternal(OS, this, TypePrinter.get(), nullptr, M,
-                         /* FromValue */ true);
+void Metadata::print(raw_ostream &OS, const Module *M) const {
+  printMetadataImpl(OS, *this, M, /* OnlyAsOperand */ false);
 }
 
 // Value::dump - allow easy printing of Values from the debugger.
@@ -3238,7 +3290,7 @@ LLVM_DUMP_METHOD
 void NamedMDNode::dump() const { print(dbgs()); }
 
 LLVM_DUMP_METHOD
-void Metadata::dump() const {
-  print(dbgs());
+void Metadata::dump(const Module *M) const {
+  print(dbgs(), M);
   dbgs() << '\n';
 }

Modified: llvm/trunk/lib/IR/Verifier.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/Verifier.cpp?rev=232275&r1=232274&r2=232275&view=diff
==============================================================================
--- llvm/trunk/lib/IR/Verifier.cpp (original)
+++ llvm/trunk/lib/IR/Verifier.cpp Sat Mar 14 15:19:36 2015
@@ -106,7 +106,7 @@ private:
   void Write(const Metadata *MD) {
     if (!MD)
       return;
-    MD->printAsOperand(OS, true, M);
+    MD->print(OS, M);
     OS << '\n';
   }
 

Modified: llvm/trunk/unittests/IR/MetadataTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/MetadataTest.cpp?rev=232275&r1=232274&r2=232275&view=diff
==============================================================================
--- llvm/trunk/unittests/IR/MetadataTest.cpp (original)
+++ llvm/trunk/unittests/IR/MetadataTest.cpp Sat Mar 14 15:19:36 2015
@@ -221,7 +221,7 @@ TEST_F(MDNodeTest, Print) {
   std::string Expected;
   {
     raw_string_ostream OS(Expected);
-    OS << "!{";
+    OS << "<" << (void *)N << "> = !{";
     C->printAsOperand(OS);
     OS << ", ";
     S->printAsOperand(OS);
@@ -241,6 +241,89 @@ TEST_F(MDNodeTest, Print) {
   EXPECT_EQ(Expected, Actual);
 }
 
+#define EXPECT_PRINTER_EQ(EXPECTED, PRINT)                                     \
+  do {                                                                         \
+    std::string Actual_;                                                       \
+    raw_string_ostream OS(Actual_);                                            \
+    PRINT;                                                                     \
+    OS.flush();                                                                \
+    std::string Expected_(EXPECTED);                                           \
+    EXPECT_EQ(Expected_, Actual_);                                             \
+  } while (false)
+
+TEST_F(MDNodeTest, PrintFromModule) {
+  Constant *C = ConstantInt::get(Type::getInt32Ty(Context), 7);
+  MDString *S = MDString::get(Context, "foo");
+  MDNode *N0 = getNode();
+  MDNode *N1 = getNode(N0);
+  MDNode *N2 = getNode(N0, N1);
+
+  Metadata *Args[] = {ConstantAsMetadata::get(C), S, nullptr, N0, N1, N2};
+  MDNode *N = MDNode::get(Context, Args);
+  Module M("test", Context);
+  NamedMDNode *NMD = M.getOrInsertNamedMetadata("named");
+  NMD->addOperand(N);
+
+  std::string Expected;
+  {
+    raw_string_ostream OS(Expected);
+    OS << "!0 = !{";
+    C->printAsOperand(OS);
+    OS << ", ";
+    S->printAsOperand(OS);
+    OS << ", null, !1, !2, !3}";
+  }
+
+  EXPECT_PRINTER_EQ(Expected, N->print(OS, &M));
+}
+
+TEST_F(MDNodeTest, PrintFromFunction) {
+  Module M("test", Context);
+  auto *FTy = FunctionType::get(Type::getVoidTy(Context), false);
+  auto *F0 = Function::Create(FTy, GlobalValue::ExternalLinkage, "F0", &M);
+  auto *F1 = Function::Create(FTy, GlobalValue::ExternalLinkage, "F1", &M);
+  auto *BB0 = BasicBlock::Create(Context, "entry", F0);
+  auto *BB1 = BasicBlock::Create(Context, "entry", F1);
+  auto *R0 = ReturnInst::Create(Context, BB0);
+  auto *R1 = ReturnInst::Create(Context, BB1);
+  auto *N0 = MDNode::getDistinct(Context, None);
+  auto *N1 = MDNode::getDistinct(Context, None);
+  R0->setMetadata("md", N0);
+  R1->setMetadata("md", N1);
+
+  EXPECT_PRINTER_EQ("!0 = distinct !{}", N0->print(OS, &M));
+  EXPECT_PRINTER_EQ("!1 = distinct !{}", N1->print(OS, &M));
+}
+
+TEST_F(MDNodeTest, PrintFromMetadataAsValue) {
+  Module M("test", Context);
+
+  auto *Intrinsic =
+      Function::Create(FunctionType::get(Type::getVoidTy(Context),
+                                         Type::getMetadataTy(Context), false),
+                       GlobalValue::ExternalLinkage, "llvm.intrinsic", &M);
+
+  auto *FTy = FunctionType::get(Type::getVoidTy(Context), false);
+  auto *F0 = Function::Create(FTy, GlobalValue::ExternalLinkage, "F0", &M);
+  auto *F1 = Function::Create(FTy, GlobalValue::ExternalLinkage, "F1", &M);
+  auto *BB0 = BasicBlock::Create(Context, "entry", F0);
+  auto *BB1 = BasicBlock::Create(Context, "entry", F1);
+  auto *N0 = MDNode::getDistinct(Context, None);
+  auto *N1 = MDNode::getDistinct(Context, None);
+  auto *MAV0 = MetadataAsValue::get(Context, N0);
+  auto *MAV1 = MetadataAsValue::get(Context, N1);
+  CallInst::Create(Intrinsic, MAV0, "", BB0);
+  CallInst::Create(Intrinsic, MAV1, "", BB1);
+
+  EXPECT_PRINTER_EQ("!0 = distinct !{}", MAV0->print(OS));
+  EXPECT_PRINTER_EQ("!1 = distinct !{}", MAV1->print(OS));
+  EXPECT_PRINTER_EQ("!0", MAV0->printAsOperand(OS, false));
+  EXPECT_PRINTER_EQ("!1", MAV1->printAsOperand(OS, false));
+  EXPECT_PRINTER_EQ("metadata !0", MAV0->printAsOperand(OS, true));
+  EXPECT_PRINTER_EQ("metadata !1", MAV1->printAsOperand(OS, true));
+}
+#undef EXPECT_PRINTER_EQ
+
 TEST_F(MDNodeTest, NullOperand) {
   // metadata !{}
   MDNode *Empty = MDNode::get(Context, None);





More information about the llvm-commits mailing list