[cfe-dev] [cfe-commits] Clang DebugInfo Patch for StopPoints.

[Chris Lattner]

On May 5, 2008, at 6:07 AM, Sanjiv.Gupta at microchip.com wrote:

>> It looks like clang is currently inconsistent here, but
>> option descriptions should not end with a ".".
>>
> I will cleanup other things here as well to remove the ending "."

Thanks!  This can be a separate patch obviously :)

>> +  CodeGenDebugInfo *TheDebugInfo;
>>
>> Please make TheDebugInfo private and add an accessor,
>> following the lead of other instance vars in CodeGenModule.
>>
>
> I am renaming the class to CGDebugInfo and files to CGDebugInfo. 
> [h,cpp]

Ok, sound sgreat.

>> #include "llvm/CodeGen/MachineModuleInfo.h"
>>
>> Is there any way to avoid bringing in MachineModuleInfo.h?
>> It is a really gross header that needs a lot of cleanup.  Can
>> you avoid #including it by forward declaring what ever you
>> need and then only #including it into CodeGenDebugInfo.cpp?
>> I think it is worth it, even if it means making 'SR' by a
>> pointer instead of holding the DISerializer by value.
>>
> Holding the 'SR' by pointer works.

Great

>>   // Don't bother if things are the same as last time.
>>   if (CurLoc == PrevLoc && PrevBB == CurBB)
>>     return;
>>
>> Why are you checking for BB equality here?
>
> This is on the lines of llvm-gcc.
> I think the reasoning behind is that a source line may result into
> multiple basic blocks and we may need to put the stoppoint in each  
> basic
> block.

Is that really desired?  It seems strange to stop multiple times even  
if there is a ?: for example.  I don't think GDB will do something  
useful.

>> Why emit a stop point on "}"?
>>
> Again, this is on the lines of llvm-gcc.
> Also, the example http://www.llvm.org/docs/SourceLevelDebugging.html
> mentions that.
>
> 1.  void foo() {
> 2.    int X = ...;
> 3.    int Y = ...;
> 4.    {
> 5.      int Z = ...;
> 6.      ...
> 7.    }
> 8.    ...
> 9.  }
>
> call void %llvm.dbg.stoppoint( uint 7, uint 2, %llvm.dbg.compile_unit*
> %llvm.dbg.compile_unit )
> call void %llvm.region.end()

I'd suggest just not doing that :)

Thanks for working on this. Debug info generation is incredibly  
important!

-Chris