[llvm-commits] Make DebugLoc not depended on DwarfWriter

[Argiris Kirtzidis]

Bill Wendling wrote:
> On Thu, Apr 30, 2009 at 12:24 PM, Evan Cheng <evan.cheng at apple.com> wrote:
>   
>> On Apr 30, 2009, at 9:10 AM, Argiris Kirtzidis wrote:
>>
>>     
>>> Currently DebugLocTuple "depends" on DwarfWriter, in that
>>> DwarfWriter::getOrCreateSourceID must be called in order to get a
>>> Source ID to put in the tuple.
>>> The attached patch makes a teenie tiny step towards a direction
>>> where DwarfWriter plays a less dominant role in debug info:
>>>       
>> Good idea.
>>
>>     
>>> -DebugLoc do (and the instruction selectors should) not depend on
>>> DwarfWriter. Source ID is more or less an "implementation detail" of
>>> DwarfWriter.
>>> -DwarfWriter should become a consumer of debug info (DebugLoc,
>>> etc.), ideally a MachineFunctionPass.
>>> -Other consumers of debug info, other than DwarfWriter, may be
>>> allowed (i.e. a JIT debugger).
>>>       
>> Ok.
>>
>>     
>>> The attached patch:
>>>
>>> -Replaces the Source ID of DebugLocTuple with a GlobalVariable * of
>>> the compile unit.
>>> -Removes DwarfWriter::getOrCreateSourceID since it isn't needed
>>> after the above change
>>> -Makes all necessary changes due to the above
>>>       
>> --- lib/CodeGen/MachineInstr.cpp        (revision 70488)
>> +++ lib/CodeGen/MachineInstr.cpp        (working copy)
>> @@ -22,6 +22,7 @@
>>  #include "llvm/Target/TargetInstrInfo.h"
>>  #include "llvm/Target/TargetInstrDesc.h"
>>  #include "llvm/Target/TargetRegisterInfo.h"
>> +#include "llvm/Analysis/DebugInfo.h"
>>  #include "llvm/Support/LeakDetector.h"
>>  #include "llvm/Support/MathExtras.h"
>>  #include "llvm/Support/Streams.h"
>> @@ -979,8 +980,10 @@
>>    if (!debugLoc.isUnknown()) {
>>      const MachineFunction *MF = getParent()->getParent();
>>      DebugLocTuple DLT = MF->getDebugLocTuple(debugLoc);
>> +    DICompileUnit CU(DLT.CompileUnit);
>> +    std::string Dir, Fn;
>>      OS << " [dbg: "
>> -       << DLT.Src  << ","
>> +       << CU.getDirectory(Dir) << '/' << CU.getFilename(Fn) << ","
>>         << DLT.Line << ","
>>         << DLT.Col  << "]";
>>    }
>>
>> It would be nice not to have to create new std::string just to print
>> them out. :-)
>>
>> Bill, can you review this?
>>
>>     
> Hi Argiris,
>
> I like this patch. Please go ahead and apply it. Thanks!
>
> As for the temporary std::string. Yeah, they are awful. I have a
> tentative patch that gets rid of the need for them, but haven't
> submitted it as it didn't cause a performance gain. . .
>
> -bw
>   

Thanks for reviewing!
Commited:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20090427/076915.html

-Argiris