[llvm] r185847 - Debug Info: clean up usage of Verify.

David Blaikie dblaikie at gmail.com
Mon Jul 8 11:55:34 PDT 2013


On Mon, Jul 8, 2013 at 11:47 AM, Manman Ren <mren at apple.com> wrote:
>
> Theoretically they should be.
> But we were using an "if" on "Verify()", which means it may be possible
> Verify() can return false.
> I can try to further replace the "if" with an "assert" and see whether all
> testing cases can pass.

I expect many should be asserts but a few will probably be handling
the "empty arrays aren't really empty & contain a single i32 0 in
them" - I'd rather address those cases explicitly rather than relying
on "Verify" or "isFoo" returning false. Something that looks more
explicitly like it's handling that one narrow case, rather than giving
the idea that we're handling all sorts of things here & ignoring a
bunch of them.

(possibly moving such a check up into DIArray might even be possible,
not sure though)

>
> Manman
>
> On Jul 8, 2013, at 11:39 AM, Eric Christopher <echristo at gmail.com> wrote:
>
> These should be asserts yes?
>
> -eric
>
> void CompileUnit::addSourceLine(DIE *Die, DIVariable V) {
>   // Verify variable.
> -  if (!V.Verify())
> +  if (!V.isVariable())
>     return;
>
>   unsigned Line = V.getLineNumber();
> @@ -259,7 +259,7 @@ void CompileUnit::addSourceLine(DIE *Die
> /// entry.
> void CompileUnit::addSourceLine(DIE *Die, DIGlobalVariable G) {
>   // Verify global variable.
> -  if (!G.Verify())
> +  if (!G.isGlobalVariable())
>     return;
>
>   unsigned Line = G.getLineNumber();
> @@ -276,7 +276,7 @@ void CompileUnit::addSourceLine(DIE *Die
> /// entry.
> void CompileUnit::addSourceLine(DIE *Die, DISubprogram SP) {
>   // Verify subprogram.
> -  if (!SP.Verify())
> +  if (!SP.isSubprogram())
>     return;
>
>   // If the line number is 0, don't add it.
> @@ -295,7 +295,7 @@ void CompileUnit::addSourceLine(DIE *Die
> /// entry.
> void CompileUnit::addSourceLine(DIE *Die, DIType Ty) {
>   // Verify type.
> -  if (!Ty.Verify())
> +  if (!Ty.isType())
>     return;
>
>   unsigned Line = Ty.getLineNumber();
> @@ -312,7 +312,7 @@ void CompileUnit::addSourceLine(DIE *Die
> /// entry.
> void CompileUnit::addSourceLine(DIE *Die, DIObjCProperty Ty) {
>   // Verify type.
> -  if (!Ty.Verify())
> +  if (!Ty.isObjCProperty())
>     return;
>
>
>
>   unsigned Line = Ty.getLineNumber();
> @@ -734,7 +734,7 @@ void CompileUnit::addToContextOwner(DIE
> /// given DIType.
> DIE *CompileUnit::getOrCreateTypeDIE(const MDNode *TyNode) {
>   DIType Ty(TyNode);
> -  if (!Ty.Verify())
> +  if (!Ty.isType())
>     return NULL;
>   DIE *TyDIE = getDIE(Ty);
>   if (TyDIE)
> @@ -773,7 +773,7 @@ DIE *CompileUnit::getOrCreateTypeDIE(con
>
> /// addType - Add a new type attribute to the specified entity.
> void CompileUnit::addType(DIE *Entity, DIType Ty, unsigned Attribute) {
> -  if (!Ty.Verify())
> +  if (!Ty.isType())
>     return;
>
>   // Check for pre-existence.
> @@ -818,7 +818,7 @@ void CompileUnit::addPubTypes(DISubprogr
>   DIArray Args = SPTy.getTypeArray();
>   for (unsigned i = 0, e = Args.getNumElements(); i != e; ++i) {
>     DIType ATy(Args.getElement(i));
> -    if (!ATy.Verify())
> +    if (!ATy.isType())
>       continue;
>     addGlobalType(ATy);
>   }
> @@ -904,7 +904,7 @@ void CompileUnit::constructTypeDIE(DIE &
>       }
>     }
>     DIType DTy = CTy.getTypeDerivedFrom();
> -    if (DTy.Verify()) {
> +    if (DTy.isType()) {
>       addType(&Buffer, DTy);
>       addUInt(&Buffer, dwarf::DW_AT_enum_class, dwarf::DW_FORM_flag, 1);
>     }
> @@ -1296,7 +1296,7 @@ void CompileUnit::createGlobalVariableDI
>     return;
>
>   DIGlobalVariable GV(N);
> -  if (!GV.Verify())
> +  if (!GV.isGlobalVariable())
>     return;
>
>   DIDescriptor GVContext = GV.getContext();
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=185847&r1=185846&r2=185847&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Mon Jul  8 13:33:29
> 2013
> @@ -934,7 +934,7 @@ void DwarfDebug::collectDeadVariables()
>       for (unsigned i = 0, e = Subprograms.getNumElements(); i != e; ++i) {
>         DISubprogram SP(Subprograms.getElement(i));
>         if (ProcessedSPNodes.count(SP) != 0) continue;
> -        if (!SP.Verify()) continue;
> +        if (!SP.isSubprogram()) continue;
>         if (!SP.isDefinition()) continue;
>         DIArray Variables = SP.getVariables();
>         if (Variables.getNumElements() == 0) continue;
> @@ -950,7 +950,7 @@ void DwarfDebug::collectDeadVariables()
>         DIE *ScopeDIE = SPCU->getDIE(SP);
>         for (unsigned vi = 0, ve = Variables.getNumElements(); vi != ve;
> ++vi) {
>           DIVariable DV(Variables.getElement(vi));
> -          if (!DV.Verify()) continue;
> +          if (!DV.isVariable()) continue;
>           DbgVariable *NewVar = new DbgVariable(DV, NULL);
>           if (DIE *VariableDIE =
>               SPCU->constructVariableDIE(NewVar, Scope->isAbstractScope()))
> @@ -1314,7 +1314,7 @@ DwarfDebug::collectVariableInfo(const Ma
>   DIArray Variables = DISubprogram(FnScope->getScopeNode()).getVariables();
>   for (unsigned i = 0, e = Variables.getNumElements(); i != e; ++i) {
>     DIVariable DV(Variables.getElement(i));
> -    if (!DV || !DV.Verify() || !Processed.insert(DV))
> +    if (!DV || !DV.isVariable() || !Processed.insert(DV))
>       continue;
>     if (LexicalScope *Scope = LScopes.findLexicalScope(DV.getContext()))
>       addScopeVariable(Scope, new DbgVariable(DV, NULL));
> @@ -1445,7 +1445,7 @@ static MDNode *getScopeNode(DebugLoc DL,
> static DebugLoc getFnDebugLoc(DebugLoc DL, const LLVMContext &Ctx) {
>   const MDNode *Scope = getScopeNode(DL, Ctx);
>   DISubprogram SP = getDISubprogram(Scope);
> -  if (SP.Verify()) {
> +  if (SP.isSubprogram()) {
>     // Check for number of operands since the compatibility is
>     // cheap here.
>     if (SP->getNumOperands() > 19)
> @@ -1513,7 +1513,7 @@ void DwarfDebug::beginFunction(const Mac
>           // The first mention of a function argument gets the
> FunctionBeginSym
>           // label, so arguments are visible when breaking at function
> entry.
>           DIVariable DV(Var);
> -          if (DV.Verify() && DV.getTag() == dwarf::DW_TAG_arg_variable &&
> +          if (DV.isVariable() && DV.getTag() == dwarf::DW_TAG_arg_variable
> &&
>               DISubprogram(getDISubprogram(DV.getContext()))
>                 .describes(MF->getFunction()))
>             LabelsBeforeInsn[MI] = FunctionBeginSym;
> @@ -1699,12 +1699,12 @@ void DwarfDebug::endFunction(const Machi
>   for (unsigned i = 0, e = AList.size(); i != e; ++i) {
>     LexicalScope *AScope = AList[i];
>     DISubprogram SP(AScope->getScopeNode());
> -    if (SP.Verify()) {
> +    if (SP.isSubprogram()) {
>       // Collect info for variables that were optimized out.
>       DIArray Variables = SP.getVariables();
>       for (unsigned i = 0, e = Variables.getNumElements(); i != e; ++i) {
>         DIVariable DV(Variables.getElement(i));
> -        if (!DV || !DV.Verify() || !ProcessedVars.insert(DV))
> +        if (!DV || !DV.isVariable() || !ProcessedVars.insert(DV))
>           continue;
>         // Check that DbgVariable for DV wasn't created earlier, when
>         // findAbstractVariable() was called for inlined instance of DV.
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h?rev=185847&r1=185846&r2=185847&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h Mon Jul  8 13:33:29 2013
> @@ -193,15 +193,15 @@ public:
>   }
>
>   bool variableHasComplexAddress()   const {
> -    assert(Var.Verify() && "Invalid complex DbgVariable!");
> +    assert(Var.isVariable() && "Invalid complex DbgVariable!");
>     return Var.hasComplexAddress();
>   }
>   bool isBlockByrefVariable()        const {
> -    assert(Var.Verify() && "Invalid complex DbgVariable!");
> +    assert(Var.isVariable() && "Invalid complex DbgVariable!");
>     return Var.isBlockByrefVariable();
>   }
>   unsigned getNumAddrElements()      const {
> -    assert(Var.Verify() && "Invalid complex DbgVariable!");
> +    assert(Var.isVariable() && "Invalid complex DbgVariable!");
>     return Var.getNumAddrElements();
>   }
>   uint64_t getAddrElement(unsigned i) const {
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>



More information about the llvm-commits mailing list