[llvm-commits] Make DebugLoc not depended on DwarfWriter

[Bill Wendling]

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