[llvm] r185484 - Debug Info: use module flag to set up Dwarf version.

Eric Christopher echristo at gmail.com
Mon Jul 8 16:15:36 PDT 2013


On Mon, Jul 8, 2013 at 4:08 PM, David Blaikie <dblaikie at gmail.com> wrote:
> On Mon, Jul 8, 2013 at 3:49 PM, Manman Ren <mren at apple.com> wrote:
>>
>> On Jul 3, 2013, at 6:31 PM, Eric Christopher wrote:
>>
>>> On Wed, Jul 3, 2013 at 6:28 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>>>
>>>> On Jul 3, 2013 5:35 PM, "Eric Christopher" <echristo at gmail.com> wrote:
>>>>>
>>>>> On Wed, Jul 3, 2013 at 5:29 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>>>>>
>>>>>> On Jul 2, 2013 7:06 PM, "Manman Ren" <mren at apple.com> wrote:
>>>>>>>
>>>>>>> Author: mren
>>>>>>> Date: Tue Jul  2 18:40:10 2013
>>>>>>> New Revision: 185484
>>>>>>>
>>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=185484&view=rev
>>>>>>> Log:
>>>>>>> Debug Info: use module flag to set up Dwarf version.
>>>>>>>
>>>>>>> Correctly handles ref_addr depending on the Dwarf version. Emit Dwarf
>>>>>>> with
>>>>>>> version from module flag.
>>>>>>>
>>>>>>> TODO: turn on/off features depending on the Dwarf version.
>>>>>>>
>>>>>>> Added:
>>>>>>>    llvm/trunk/test/DebugInfo/version.ll
>>>>>>> Modified:
>>>>>>>    llvm/trunk/include/llvm/CodeGen/AsmPrinter.h
>>>>>>>    llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp
>>>>>>>    llvm/trunk/lib/CodeGen/AsmPrinter/DIE.h
>>>>>>>    llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>>>>>>>    llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>>>>>>>
>>>>>>> Modified: llvm/trunk/include/llvm/CodeGen/AsmPrinter.h
>>>>>>> URL:
>>>>>>>
>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/AsmPrinter.h?rev=185484&r1=185483&r2=185484&view=diff
>>>>>>>
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- llvm/trunk/include/llvm/CodeGen/AsmPrinter.h (original)
>>>>>>> +++ llvm/trunk/include/llvm/CodeGen/AsmPrinter.h Tue Jul  2 18:40:10
>>>>>>> 2013
>>>>>>> @@ -121,6 +121,8 @@ namespace llvm {
>>>>>>>   public:
>>>>>>>     virtual ~AsmPrinter();
>>>>>>>
>>>>>>> +    const DwarfDebug *getDwarfDebug() const { return DD; }
>>>>>>> +
>>>>>>>     /// isVerbose - Return true if assembly output should contain
>>>>>>> comments.
>>>>>>>     ///
>>>>>>>     bool isVerbose() const { return VerboseAsm; }
>>>>>>>
>>>>>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp
>>>>>>> URL:
>>>>>>>
>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp?rev=185484&r1=185483&r2=185484&view=diff
>>>>>>>
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp (original)
>>>>>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp Tue Jul  2 18:40:10 2013
>>>>>>> @@ -12,6 +12,7 @@
>>>>>>>
>>>>>>>
>>>>>>> //===----------------------------------------------------------------------===//
>>>>>>>
>>>>>>> #include "DIE.h"
>>>>>>> +#include "DwarfDebug.h"
>>>>>>> #include "llvm/ADT/Twine.h"
>>>>>>> #include "llvm/CodeGen/AsmPrinter.h"
>>>>>>> #include "llvm/IR/DataLayout.h"
>>>>>>> @@ -331,6 +332,16 @@ void DIEEntry::EmitValue(AsmPrinter *AP,
>>>>>>>   AP->EmitInt32(Entry->getOffset());
>>>>>>> }
>>>>>>>
>>>>>>> +unsigned DIEEntry::getRefAddrSize(AsmPrinter *AP) {
>>>>>>> +  // DWARF4: References that use the attribute form DW_FORM_ref_addr
>>>>>>> are
>>>>>>> +  // specified to be four bytes in the DWARF 32-bit format and eight
>>>>>>> bytes
>>>>>>> +  // in the DWARF 64-bit format, while DWARF Version 2 specifies that
>>>>>>> such
>>>>>>> +  // references have the same size as an address on the target system.
>>>>>>> +  if (AP->getDwarfDebug()->getDwarfVersion() == 2)
>>>>>>> +    return AP->getDataLayout().getPointerSize();
>>>>>>> +  return sizeof(int32_t);
>>>>>>> +}
>>>>>>
>>>>>> This seems like a slightly strange place for this function - it also
>>>>>> doesn't
>>>>>> need any access to private members of DIEEntry. As it's a generic debug
>>>>>> info
>>>>>> utility function, could it go somewhere more central than DIEEntry? (If
>>>>>> its
>>>>>> needed from other places, which it seems to be)
>>>>>>
>>>>>>> +
>>>>>>> #ifndef NDEBUG
>>>>>>> void DIEEntry::print(raw_ostream &O) const {
>>>>>>>   O << format("Die: 0x%lx", (long)(intptr_t)Entry);
>>>>>>>
>>>>>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DIE.h
>>>>>>> URL:
>>>>>>>
>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DIE.h?rev=185484&r1=185483&r2=185484&view=diff
>>>>>>>
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DIE.h (original)
>>>>>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DIE.h Tue Jul  2 18:40:10 2013
>>>>>>> @@ -365,9 +365,13 @@ namespace llvm {
>>>>>>>     /// SizeOf - Determine size of debug information entry in bytes.
>>>>>>>     ///
>>>>>>>     virtual unsigned SizeOf(AsmPrinter *AP, unsigned Form) const {
>>>>>>> -      return sizeof(int32_t);
>>>>>>> +      return Form == dwarf::DW_FORM_ref_addr ? getRefAddrSize(AP) :
>>>>>>> +                                               sizeof(int32_t);
>>>>>>>     }
>>>>>>>
>>>>>>> +    /// Returns size of a ref_addr entry.
>>>>>>> +    static unsigned getRefAddrSize(AsmPrinter *AP);
>>>>>>> +
>>>>>>>     // Implement isa/cast/dyncast.
>>>>>>>     static bool classof(const DIEValue *E) { return E->getType() ==
>>>>>>> isEntry; }
>>>>>>>
>>>>>>>
>>>>>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>>>>>>> URL:
>>>>>>>
>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=185484&r1=185483&r2=185484&view=diff
>>>>>>>
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
>>>>>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Tue Jul  2
>>>>>>> 18:40:10
>>>>>>> 2013
>>>>>>> @@ -162,6 +162,21 @@ DIType DbgVariable::getType() const {
>>>>>>>
>>>>>>> } // end llvm namespace
>>>>>>>
>>>>>>> +/// Return Dwarf Version by checking module flags.
>>>>>>> +static unsigned getDwarfVersionFromModule(const Module *M) {
>>>>>>> +  SmallVector<Module::ModuleFlagEntry, 8> ModuleFlags;
>>>>>>> +  M->getModuleFlagsMetadata(ModuleFlags);
>>>>>>> +  for (unsigned I = 0, E = ModuleFlags.size(); I < E; ++I) {
>>>>>>> +    const Module::ModuleFlagEntry &MFE = ModuleFlags[I];
>>>>>>> +    StringRef Key = MFE.Key->getString();
>>>>>>> +    Value *Val = MFE.Val;
>>>>>>> +
>>>>>>> +    if (Key == "Dwarf Version")
>>>>>>> +      return cast<ConstantInt>(Val)->getZExtValue();
>>>>>>> +  }
>>>>>>> +  return dwarf::DWARF_VERSION;
>>>>>>
>>>>>> Do we not have a better way to query module flags? Perhaps we should add
>>>>>> one?
>>>>>>
>>>>>> Is there any need to support debug info generation for modules that lack
>>>>>> this flag? Seems we could just consider that to be invalid?
>>>>>>
>>>>>
>>>>> Icky. Either we need to autoupgrade modules that come in to specify a
>>>>> dwarf version or leave this sort of thing around. Admittedly if we
>>>>> stored it in the CU we could call it an incompatible debug info change
>>>>> :)
>>>>
>>>> I think we could still make that claim even if its a module flag. If you
>>>> have debug info metadata and no flag: invalid. If you don't have any debug
>>>> info metadata, we shouldn't care what the flag value is,
>>>>
>>>
>>> Sure. Can do. I am in favor of Module having a function to look up an
>>> attribute marked by key. I assume that was just an oversight.
>>>
>>> Manman: Can you please make all of these changes?
>>
>> I can definitely add a helper function in Module to query a module flag given a key.
>> If we say a module without "Dwarf Version" flag is invalid, we need to update all the existing test cases.
>
> I don't think we have to do that... we can still have a helper
> function that queries a given flag & just returns some "nonvalue" or
> default value if the module metadata isn't present.
>
>> If we are going to use a metadata field in each DICompileUnit in the long run, I would prefer
>> to switch to that now.
>
> Due to a lack of clear recollection of prior discussions on the
> subject, I'm less clear on whether there's any overwhelming reason not
> to do this, so I can't really veto it.
>

Shrug. I think we'll want it in the future, but we can wait until
then. No one in their right mind is depending upon the debug
information api staying compatible.

-eric

>>
>> Manman
>>
>>> Thanks.
>>>
>>> -eric
>>>
>>>>>
>>>>> -eric
>>>>>
>>>>>>> +}
>>>>>>> +
>>>>>>> DwarfDebug::DwarfDebug(AsmPrinter *A, Module *M)
>>>>>>>   : Asm(A), MMI(Asm->MMI), FirstCU(0),
>>>>>>>     AbbreviationsSet(InitAbbreviationsSetSize),
>>>>>>> @@ -204,6 +219,8 @@ DwarfDebug::DwarfDebug(AsmPrinter *A, Mo
>>>>>>>   else
>>>>>>>     HasSplitDwarf = SplitDwarf == Enable ? true : false;
>>>>>>>
>>>>>>> +  DwarfVersion = getDwarfVersionFromModule(MMI->getModule());
>>>>>>> +
>>>>>>>   {
>>>>>>>     NamedRegionTimer T(DbgTimerName, DWARFGroupName,
>>>>>>> TimePassesIsEnabled);
>>>>>>>     beginModule();
>>>>>>> @@ -1900,7 +1917,8 @@ void DwarfDebug::emitDIE(DIE *Die, std::
>>>>>>>         DwarfUnits &Holder = useSplitDwarf() ? SkeletonHolder :
>>>>>>> InfoHolder;
>>>>>>>         Addr += Holder.getCUOffset(Origin->getCompileUnit());
>>>>>>>       }
>>>>>>> -      Asm->EmitInt32(Addr);
>>>>>>> +      Asm->OutStreamer.EmitIntValue(Addr,
>>>>>>> +          Form == dwarf::DW_FORM_ref_addr ?
>>>>>>> DIEEntry::getRefAddrSize(Asm)
>>>>>>> : 4);
>>>>>>>       break;
>>>>>>>     }
>>>>>>>     case dwarf::DW_AT_ranges: {
>>>>>>> @@ -1984,7 +2002,7 @@ void DwarfUnits::emitUnits(DwarfDebug *D
>>>>>>>     Asm->OutStreamer.AddComment("Length of Compilation Unit Info");
>>>>>>>     Asm->EmitInt32(ContentSize);
>>>>>>>     Asm->OutStreamer.AddComment("DWARF version number");
>>>>>>> -    Asm->EmitInt16(dwarf::DWARF_VERSION);
>>>>>>> +    Asm->EmitInt16(DD->getDwarfVersion());
>>>>>>>     Asm->OutStreamer.AddComment("Offset Into Abbrev. Section");
>>>>>>>
>>>>>>>
>>>>>>> Asm->EmitSectionOffset(Asm->GetTempSymbol(ASection->getLabelBeginName()),
>>>>>>>                            ASectionSym);
>>>>>>> @@ -2225,7 +2243,7 @@ void DwarfDebug::emitDebugPubnames() {
>>>>>>>     Asm->OutStreamer.EmitLabel(Asm->GetTempSymbol("pubnames_begin",
>>>>>>> ID));
>>>>>>>
>>>>>>>     Asm->OutStreamer.AddComment("DWARF Version");
>>>>>>> -    Asm->EmitInt16(dwarf::DWARF_VERSION);
>>>>>>> +    Asm->EmitInt16(DwarfVersion);
>>>>>>>
>>>>>>>     Asm->OutStreamer.AddComment("Offset of Compilation Unit Info");
>>>>>>>
>>>>>>> Asm->EmitSectionOffset(Asm->GetTempSymbol(ISec->getLabelBeginName(),
>>>>>>> ID),
>>>>>>> @@ -2272,7 +2290,7 @@ void DwarfDebug::emitDebugPubTypes() {
>>>>>>>
>>>>>>> TheCU->getUniqueID()));
>>>>>>>
>>>>>>>     if (Asm->isVerbose()) Asm->OutStreamer.AddComment("DWARF
>>>>>>> Version");
>>>>>>> -    Asm->EmitInt16(dwarf::DWARF_VERSION);
>>>>>>> +    Asm->EmitInt16(DwarfVersion);
>>>>>>>
>>>>>>>     Asm->OutStreamer.AddComment("Offset of Compilation Unit Info");
>>>>>>>     const MCSection *ISec =
>>>>>>> Asm->getObjFileLowering().getDwarfInfoSection();
>>>>>>> @@ -2553,7 +2571,7 @@ void DwarfDebug::emitDebugInlineInfo() {
>>>>>>>   Asm->OutStreamer.EmitLabel(Asm->GetTempSymbol("debug_inlined_begin",
>>>>>>> 1));
>>>>>>>
>>>>>>>   Asm->OutStreamer.AddComment("Dwarf Version");
>>>>>>> -  Asm->EmitInt16(dwarf::DWARF_VERSION);
>>>>>>> +  Asm->EmitInt16(DwarfVersion);
>>>>>>>   Asm->OutStreamer.AddComment("Address Size (in bytes)");
>>>>>>>   Asm->EmitInt8(Asm->getDataLayout().getPointerSize());
>>>>>>>
>>>>>>>
>>>>>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>>>>>>> URL:
>>>>>>>
>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h?rev=185484&r1=185483&r2=185484&view=diff
>>>>>>>
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h (original)
>>>>>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h Tue Jul  2 18:40:10
>>>>>>> 2013
>>>>>>> @@ -407,6 +407,8 @@ class DwarfDebug {
>>>>>>>   bool HasDwarfAccelTables;
>>>>>>>   bool HasSplitDwarf;
>>>>>>>
>>>>>>> +  unsigned DwarfVersion;
>>>>>>> +
>>>>>>>   // Separated Dwarf Variables
>>>>>>>   // In general these will all be for bits that are left in the
>>>>>>>   // original object file, rather than things that are meant
>>>>>>> @@ -650,6 +652,9 @@ public:
>>>>>>>   /// \brief Returns whether or not to change the current debug info
>>>>>>> for
>>>>>>> the
>>>>>>>   /// split dwarf proposal support.
>>>>>>>   bool useSplitDwarf() { return HasSplitDwarf; }
>>>>>>> +
>>>>>>> +  /// Returns the Dwarf Version.
>>>>>>> +  unsigned getDwarfVersion() const { return DwarfVersion; }
>>>>>>> };
>>>>>>> } // End of namespace llvm
>>>>>>>
>>>>>>>
>>>>>>> Added: llvm/trunk/test/DebugInfo/version.ll
>>>>>>> URL:
>>>>>>>
>>>>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/DebugInfo/version.ll?rev=185484&view=auto
>>>>>>>
>>>>>>>
>>>>>>> ==============================================================================
>>>>>>> --- llvm/trunk/test/DebugInfo/version.ll (added)
>>>>>>> +++ llvm/trunk/test/DebugInfo/version.ll Tue Jul  2 18:40:10 2013
>>>>>>> @@ -0,0 +1,27 @@
>>>>>>> +; RUN: llc -O0 < %s | FileCheck %s
>>>>>>> +; Make sure we are generating DWARF version 3 when module flag says
>>>>>>> so.
>>>>>>> +; CHECK: .short        3 ## DWARF version number
>>>>>>> +
>>>>>>> +define i32 @main() #0 {
>>>>>>> +entry:
>>>>>>> +  %retval = alloca i32, align 4
>>>>>>> +  store i32 0, i32* %retval
>>>>>>> +  ret i32 0, !dbg !10
>>>>>>> +}
>>>>>>> +
>>>>>>> +attributes #0 = { nounwind uwtable "less-precise-fpmad"="false"
>>>>>>> "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf"="true"
>>>>>>> "no-infs-fp-math"="false" "no-nans-fp-math"="false"
>>>>>>> "unsafe-fp-math"="false"
>>>>>>> "use-soft-float"="false" }
>>>>>>> +
>>>>>>> +!llvm.dbg.cu = !{!0}
>>>>>>> +!llvm.module.flags = !{!9}
>>>>>>> +
>>>>>>> +!0 = metadata !{i32 786449, metadata !1, i32 12, metadata !"clang
>>>>>>> version
>>>>>>> 3.4 (trunk 185475)", i1 false, metadata !"", i32 0, metadata !2,
>>>>>>> metadata
>>>>>>> !2, metadata !3, metadata !2, metadata !2, metadata !""} ; [
>>>>>>> DW_TAG_compile_unit ]
>>>>>>> +!1 = metadata !{metadata !"CodeGen/dwarf-version.c", metadata !"test"}
>>>>>>> +!2 = metadata !{i32 0}
>>>>>>> +!3 = metadata !{metadata !4}
>>>>>>> +!4 = metadata !{i32 786478, metadata !1, metadata !5, metadata
>>>>>>> !"main",
>>>>>>> metadata !"main", metadata !"", i32 6, metadata !6, i1 false, i1 true,
>>>>>>> i32
>>>>>>> 0, i32 0, null, i32 256, i1 false, i32 ()* @main, null, null, metadata
>>>>>>> !2,
>>>>>>> i32 6} ; [ DW_TAG_subprogram ] [line 6] [def] [main]
>>>>>>> +!5 = metadata !{i32 786473, metadata !1}          ; [ DW_TAG_file_type
>>>>>>> ]
>>>>>>> +!6 = metadata !{i32 786453, i32 0, i32 0, metadata !"", i32 0, i64 0,
>>>>>>> i64
>>>>>>> 0, i64 0, i32 0, null, metadata !7, i32 0, i32 0} ; [
>>>>>>> DW_TAG_subroutine_type
>>>>>>> ] [line 0, size 0, align 0, offset 0] [from ]
>>>>>>> +!7 = metadata !{metadata !8}
>>>>>>> +!8 = metadata !{i32 786468, null, null, metadata !"int", i32 0, i64
>>>>>>> 32,
>>>>>>> i64 32, i64 0, i32 0, i32 5} ; [ DW_TAG_base_type ] [int] [line 0, size
>>>>>>> 32,
>>>>>>> align 32, offset 0, enc DW_ATE_signed]
>>>>>>> +!9 = metadata !{i32 2, metadata !"Dwarf Version", i32 3}
>>>>>>> +!10 = metadata !{i32 7, i32 0, metadata !4, null}
>>>>>>>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> 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