[llvm-commits] [llvm] r103135 - /llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

[Devang Patel]

Dan,

This does not make sense. See below...

On May 5, 2010, at 4:41 PM, Dan Gohman wrote:

> Author: djg
> Date: Wed May  5 18:41:32 2010
> New Revision: 103135
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=103135&view=rev
> Log:
> Emit debug info for MachineInstrs with unknown debug locations, instead
> of just letting them inherit the debug locations of adjacent instructions.
> 
> Debug info should aim to be either accurate or absent.
> 
> Modified:
>    llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> 
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=103135&r1=103134&r2=103135&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Wed May  5 18:41:32 2010
> @@ -2150,8 +2150,20 @@
> void DwarfDebug::beginScope(const MachineInstr *MI) {
>   // Check location.
>   DebugLoc DL = MI->getDebugLoc();
> -  if (DL.isUnknown())
> +  if (DL.isUnknown()) {
> +    // This instruction has no debug location. If the preceding instruction
> +    // did, emit debug location information to indicate that the debug
> +    // location is now unknown.
> +    MCSymbol *Label = NULL;
> +    if (DL == PrevInstLoc)
> +      Label = PrevLabel;

Here, Label is not used afterwords.

> +    else {
> +      Label = recordSourceLine(DL.getLine(), DL.getCol(), 0);

If DL is unknown then DL.getLine(), DL.getCol() are not useful to record source location.
> +      PrevInstLoc = DL;
> +      PrevLabel = Label;

 And here PrevInstLoc and PrevLable were updated when a label for valid location was emitted. Instead you're overriding valid PrevInstLoc with Unknown debug location.

> +    }
>     return;
> +  }


>   MDNode *Scope = DL.getScope(Asm->MF->getFunction()->getContext());
> 
> @@ -2564,23 +2576,28 @@
>   StringRef Dir;
>   StringRef Fn;
> 
> -  DIDescriptor Scope(S);
> -  if (Scope.isCompileUnit()) {
> -    DICompileUnit CU(S);
> -    Dir = CU.getDirectory();
> -    Fn = CU.getFilename();
> -  } else if (Scope.isSubprogram()) {
> -    DISubprogram SP(S);
> -    Dir = SP.getDirectory();
> -    Fn = SP.getFilename();
> -  } else if (Scope.isLexicalBlock()) {
> -    DILexicalBlock DB(S);
> -    Dir = DB.getDirectory();
> -    Fn = DB.getFilename();
> -  } else
> -    assert(0 && "Unexpected scope info");
> +  unsigned Src = 1;
> +  if (S) {
> +    DIDescriptor Scope(S);
> +
> +    if (Scope.isCompileUnit()) {
> +      DICompileUnit CU(S);
> +      Dir = CU.getDirectory();
> +      Fn = CU.getFilename();
> +    } else if (Scope.isSubprogram()) {
> +      DISubprogram SP(S);
> +      Dir = SP.getDirectory();
> +      Fn = SP.getFilename();
> +    } else if (Scope.isLexicalBlock()) {
> +      DILexicalBlock DB(S);
> +      Dir = DB.getDirectory();
> +      Fn = DB.getFilename();
> +    } else
> +      assert(0 && "Unexpected scope info");
> +
> +    Src = GetOrCreateSourceID(Dir, Fn);
> +  }

I am not sure what exactly this part achieves other then increasing indentation.
> 
> -  unsigned Src = GetOrCreateSourceID(Dir, Fn);
>   MCSymbol *Label = MMI->getContext().CreateTempSymbol();
>   Lines.push_back(SrcLineInfo(Line, Col, Src, Label));
> 
> @@ -2967,8 +2984,6 @@
>       MCSymbol *Label = LineInfo.getLabel();
>       if (!Label->isDefined()) continue; // Not emitted, in dead code.
> 
> -      if (LineInfo.getLine() == 0) continue;
> -
>       if (Asm->isVerbose()) {
>         std::pair<unsigned, unsigned> SrcID =
>           getSourceDirectoryAndFileIds(LineInfo.getSourceID());

Can you send me small test case where this patch improves debug info ?
Thanks,
-
Devang
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20100505/b6312a70/attachment.html>