IR: Take advantage of -verify checks for MDExpression

Now that we check `MDExpression` during `-verify` (r232299), make
the `DIExpression` wrapper more strict:

  - remove redundant checks in `DebugInfoVerifier`,
  - overload `get()` to `cast_or_null<MDExpression>` (superseding
    `getRaw()`),
  - stop checking for null in any accessor, and
  - remove `DIExpression::Verify()` entirely in favour of
    `MDExpression::isValid()`.

There is still some logic in this class, mostly to do with high-level
iterators; I'll defer cleaning up those until the rest of the wrappers
are similarly strict.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@232412 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/CodeGen/MachineInstr.h b/include/llvm/CodeGen/MachineInstr.h
index f9cdbf05cb6f68844c5fc59218d22133e6e24cb0..333dcdb3effa6dc0b37b8d0733b997cf22ee4601 100644 (file)
--- a/include/llvm/CodeGen/MachineInstr.h
+++ b/include/llvm/CodeGen/MachineInstr.h
@@ -257,9 +257,7 @@ public:
   /// this DBG_VALUE instruction.
   DIExpression getDebugExpression() const {
     assert(isDebugValue() && "not a DBG_VALUE");
-    DIExpression Expr(getOperand(3).getMetadata());
-    assert(Expr.Verify() && "not a DIExpression");
-    return Expr;
+    return cast<MDExpression>(getOperand(3).getMetadata());
   }
 
   /// emitError - Emit an error referring to the source location of this

diff --git a/include/llvm/CodeGen/MachineInstrBuilder.h b/include/llvm/CodeGen/MachineInstrBuilder.h
index 8859b6a019ea4359e33e29b8c6c26c8811b71d87..e6cb4945debe0aec6c01e2517cb1df55245ada01 100644 (file)
--- a/include/llvm/CodeGen/MachineInstrBuilder.h
+++ b/include/llvm/CodeGen/MachineInstrBuilder.h
@@ -356,7 +356,7 @@ inline MachineInstrBuilder BuildMI(MachineFunction &MF, DebugLoc DL,
                                    unsigned Reg, unsigned Offset,
                                    const MDNode *Variable, const MDNode *Expr) {
   assert(DIVariable(Variable).Verify() && "not a DIVariable");
-  assert(DIExpression(Expr).Verify() && "not a DIExpression");
+  assert(DIExpression(Expr)->isValid() && "not a DIExpression");
   if (IsIndirect)
     return BuildMI(MF, DL, MCID)
         .addReg(Reg, RegState::Debug)
@@ -383,7 +383,7 @@ inline MachineInstrBuilder BuildMI(MachineBasicBlock &BB,
                                    unsigned Reg, unsigned Offset,
                                    const MDNode *Variable, const MDNode *Expr) {
   assert(DIVariable(Variable).Verify() && "not a DIVariable");
-  assert(DIExpression(Expr).Verify() && "not a DIExpression");
+  assert(DIExpression(Expr)->isValid() && "not a DIExpression");
   MachineFunction &MF = *BB.getParent();
   MachineInstr *MI =
       BuildMI(MF, DL, MCID, IsIndirect, Reg, Offset, Variable, Expr);

diff --git a/include/llvm/IR/DebugInfo.h b/include/llvm/IR/DebugInfo.h
index 5ab8a297465c6dc1317e9c7996358a35ac9408dc..c9189e551d54c6f7a49c633d629e7a48cc842753 100644 (file)
--- a/include/llvm/IR/DebugInfo.h
+++ b/include/llvm/IR/DebugInfo.h
@@ -989,21 +989,24 @@ public:
 /// FIXME: Instead of DW_OP_plus taking an argument, this should use DW_OP_const
 /// and have DW_OP_plus consume the topmost elements on the stack.
 class DIExpression : public DIDescriptor {
-  MDExpression *getRaw() const { return dyn_cast_or_null<MDExpression>(get()); }
-
 public:
   explicit DIExpression(const MDNode *N = nullptr) : DIDescriptor(N) {}
   DIExpression(const MDExpression *N) : DIDescriptor(N) {}
 
-  bool Verify() const;
+  MDExpression *get() const {
+    return cast_or_null<MDExpression>(DIDescriptor::get());
+  }
+  operator MDExpression *() const { return get(); }
+  MDExpression *operator->() const { return get(); }
+
+  // Don't call this.  Call isValid() directly.
+  bool Verify() const = delete;
 
   /// \brief Return the number of elements in the complex expression.
-  unsigned getNumElements() const { RETURN_FROM_RAW(N->getNumElements(), 0); }
+  unsigned getNumElements() const { return get()->getNumElements(); }
 
   /// \brief return the Idx'th complex address element.
-  uint64_t getElement(unsigned I) const {
-    return cast<MDExpression>(get())->getElement(I);
-  }
+  uint64_t getElement(unsigned I) const { return get()->getElement(I); }
 
   /// \brief Return whether this is a piece of an aggregate variable.
   bool isBitPiece() const;
@@ -1069,8 +1072,8 @@ public:
     }
   };
 
-  iterator begin() const { return cast<MDExpression>(get())->elements_begin(); }
-  iterator end() const { return cast<MDExpression>(get())->elements_end(); }
+  iterator begin() const { return get()->elements_begin(); }
+  iterator end() const { return get()->elements_end(); }
 };
 
 /// \brief This object holds location information.

diff --git a/lib/CodeGen/AsmPrinter/DebugLocEntry.h b/lib/CodeGen/AsmPrinter/DebugLocEntry.h
index 6f30d294f7187784f75bfb07a9d0dc206252f48b..6914bbe9a5946cc2ecdd1b844ef4127328b116cd 100644 (file)
--- a/lib/CodeGen/AsmPrinter/DebugLocEntry.h
+++ b/lib/CodeGen/AsmPrinter/DebugLocEntry.h
@@ -43,7 +43,7 @@ public:
     Value(const MDNode *Var, const MDNode *Expr, MachineLocation Loc)
         : Variable(Var), Expression(Expr), EntryKind(E_Location), Loc(Loc) {
       assert(DIVariable(Var).Verify());
-      assert(DIExpression(Expr).Verify());
+      assert(DIExpression(Expr)->isValid());
     }
 
     /// The variable to which this location entry corresponds.

diff --git a/lib/CodeGen/AsmPrinter/DwarfDebug.h b/lib/CodeGen/AsmPrinter/DwarfDebug.h
index 6e462bf06d05c0e252860bc3213ecf798bd031cf..952da04a4b4126a7929a49d4fe01cba57a62a7ec 100644 (file)
--- a/lib/CodeGen/AsmPrinter/DwarfDebug.h
+++ b/lib/CodeGen/AsmPrinter/DwarfDebug.h
@@ -88,7 +88,8 @@ public:
     : Var(V), Expr(1, E), TheDIE(nullptr), DotDebugLocOffset(~0U),
       MInsn(nullptr), DD(DD) {
     FrameIndex.push_back(FI);
-    assert(Var.Verify() && E.Verify());
+    assert(Var.Verify());
+    assert(!E || E->isValid());
   }
 
   /// Construct a DbgVariable from a DEBUG_VALUE.

diff --git a/lib/IR/DebugInfo.cpp b/lib/IR/DebugInfo.cpp
index 148fa2101c0340df9b22eb28b4ffc62e32ad814b..3c7cf9e1b8d22bcdf62ba1cced7e929fb23f93bf 100644 (file)
--- a/lib/IR/DebugInfo.cpp
+++ b/lib/IR/DebugInfo.cpp
@@ -92,7 +92,7 @@ bool DIDescriptor::Verify() const {
           DIObjCProperty(DbgNode).Verify() ||
           DITemplateTypeParameter(DbgNode).Verify() ||
           DITemplateValueParameter(DbgNode).Verify() ||
-          DIImportedEntity(DbgNode).Verify() || DIExpression(DbgNode).Verify());
+          DIImportedEntity(DbgNode).Verify());
 }
 
 static Metadata *getField(const MDNode *DbgNode, unsigned Elt) {
@@ -402,12 +402,6 @@ bool DIVariable::Verify() const {
   return isTypeRef(N->getType());
 }
 
-bool DIExpression::Verify() const {
-  // FIXME: This should return false if it's null!
-  auto *N = getRaw();
-  return !N || N->isValid();
-}
-
 bool DILocation::Verify() const { return getRaw(); }
 bool DINameSpace::Verify() const { return getRaw(); }
 bool DIFile::Verify() const { return getRaw(); }

diff --git a/lib/IR/Verifier.cpp b/lib/IR/Verifier.cpp
index ba33d792cb20243a94a46e8be769dc3041847eba..cc5fa690d58e8502d7bd9bb0253fa1b023c5c16b 100644 (file)
--- a/lib/IR/Verifier.cpp
+++ b/lib/IR/Verifier.cpp
@@ -3078,15 +3078,11 @@ void DebugInfoVerifier::processCallInst(DebugInfoFinder &Finder,
       case Intrinsic::dbg_declare: {
         auto *DDI = cast<DbgDeclareInst>(&CI);
         Finder.processDeclare(*M, DDI);
-        if (auto E = DDI->getExpression())
-          Assert(DIExpression(E).Verify(), "DIExpression does not Verify!", E);
         break;
       }
       case Intrinsic::dbg_value: {
         auto *DVI = cast<DbgValueInst>(&CI);
         Finder.processValue(*M, DVI);
-        if (auto E = DVI->getExpression())
-          Assert(DIExpression(E).Verify(), "DIExpression does not Verify!", E);
         break;
       }
       default: