From 3d5527fb43b1cc3147ea9842c41263a5a156d6ac Mon Sep 17 00:00:00 2001 From: "Duncan P. N. Exon Smith" Date: Mon, 16 Mar 2015 21:03:55 +0000 Subject: [PATCH] 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` (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 --- include/llvm/CodeGen/MachineInstr.h | 4 +--- include/llvm/CodeGen/MachineInstrBuilder.h | 4 ++-- include/llvm/IR/DebugInfo.h | 21 ++++++++++++--------- lib/CodeGen/AsmPrinter/DebugLocEntry.h | 2 +- lib/CodeGen/AsmPrinter/DwarfDebug.h | 3 ++- lib/IR/DebugInfo.cpp | 8 +------- lib/IR/Verifier.cpp | 4 ---- 7 files changed, 19 insertions(+), 27 deletions(-) diff --git a/include/llvm/CodeGen/MachineInstr.h b/include/llvm/CodeGen/MachineInstr.h index f9cdbf05cb6..333dcdb3eff 100644 --- 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(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 8859b6a019e..e6cb4945deb 100644 --- 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 5ab8a297465..c9189e551d5 100644 --- 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(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(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(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(get())->elements_begin(); } - iterator end() const { return cast(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 6f30d294f71..6914bbe9a59 100644 --- 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 6e462bf06d0..952da04a4b4 100644 --- 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 148fa2101c0..3c7cf9e1b8d 100644 --- 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 ba33d792cb2..cc5fa690d58 100644 --- 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(&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(&CI); Finder.processValue(*M, DVI); - if (auto E = DVI->getExpression()) - Assert(DIExpression(E).Verify(), "DIExpression does not Verify!", E); break; } default: -- 2.34.1