[PATCH] D26769: [IR] Remove the DIExpression field from DIGlobalVariable.

[Adrian Prantl via llvm-commits]

> On Nov 18, 2016, at 10:14 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Fri, Nov 18, 2016 at 10:11 AM Adrian Prantl <aprantl at apple.com <mailto:aprantl at apple.com>> wrote:
> 
> > On Nov 18, 2016, at 9:55 AM, Duncan P. N. Exon Smith <dexonsmith at apple.com <mailto:dexonsmith at apple.com>> wrote:
> >
> > No comment on whether we need an offset/etc.; but assuming we do want that some day, it might be more efficient to add an offset field to DIGlobalVariableExpr:
> >
> > DIGlobalVariableExpr(value: DW_OP_const 42, offset: 32 bits) -> DIGlobalVariable(name: foo)
> 
> I'm very much against this. We probably want to share as much infrastructure as possible between global and local variables. Putting special support into DIGlobalVariableExpresssion moves us farther away from that goal.
> 
> With DIExpression (which is really a DWARF expression) we have an extensible representation for all sorts of operations that may be necessary to reconstruct a source variable. I'm fine with improving the syntax to make it more readable, but I would not want to make it any less generic.
> 
> I'm still pretty confused about how any of this works correctly - before even trying to wrap my head around how these things would be generic for pieces of a variable. (we don't use the local variable descriptions this way - each description is a complete description of the variable (as complete as we know), possibly overlapping in time/instruction region (do we support that? probably not right now?))

(See also my other mail.)
We have one @dbg.value(var, expr) intrinsic for each piece, and there can be multiple of these over time.

Roughly summarized, DwarfDebug::buildLocationList() goes through all DBG_VALUEs sorts them, cuts the regions into the into the smallest necessary segments and then builds a location list that contains, for each segment in the variable's live range, the locations of each of the pieces and their respective DWARF expressions.

-- adrian

> 
> Not sure what generality you're considering drawing from this or how it might look. I think the same concerns (how do we manipulate expressions when optimizing, etc) I was describing apply for local variables too, so I wouldn't see it as less general.
> 
> - David
>  
> 
> -- adrian
> 
> >
> >
> >> On 2016-Nov-18, at 09:51, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
> >>
> >> Would it be reasonable/useful to add another level of indirection while we're here - that would describe which part of the variable is being described by this location:
> >>
> >> DIGlobalVariableExpr(value: DW_OP_const 42) -> DIGlobalVariableFragment(offset: 32 bits) -> DIGlobalVariable(name: foo)
> >>
> >> (just guessing/pretending there's a DW_OP_const for constant values - I forget how it all works, but purely for demonstration purposes)
> >>
> >> That way code manipulating DIGlobalVariableExprs wouldn't need to have explicit knowledge of which part of the variable is being described here. (I suppose that could end up with us creating a bottom-up expression tree in metadata, which probably isn't ideal (too heavyweight))
> >>
> >> Perhaps keeping the bit_piece parts out of the DWARF expression bytes and in a separate field of the DIGlobalVariableExpr?
> >>
> >> DIGlobalVariableExpr(offset: 32, value: DW_OP_const 42)
> >>
> >> Then at least when a variable moves around (does that happen? maybe? - I suppose something like asan, smooshing everything into a single alloca would move something around without splitting it up) the expression can be updated relatively simply without unpacking through a DW_OP_bit_piece, etc?
> >>
> >> Maybe this doesn't matter - Ih aven't looked at much of the expression/location handling code.
> >>
> >> On Fri, Nov 18, 2016 at 9:32 AM Adrian Prantl <aprantl at apple.com <mailto:aprantl at apple.com>> wrote:
> >> aprantl removed rL LLVM as the repository for this revision.
> >> aprantl updated this revision to Diff 78544.
> >> aprantl added a comment.
> >>
> >> Improved type-safety by adding a DIGlobalVarExpr pointer union.
> >>
> >>
> >> https://reviews.llvm.org/D26769 <https://reviews.llvm.org/D26769>
> >>
> >> Files:
> >>  include/llvm/Bitcode/LLVMBitCodes.h
> >>  include/llvm/IR/DIBuilder.h
> >>  include/llvm/IR/DebugInfo.h
> >>  include/llvm/IR/DebugInfoMetadata.h
> >>  include/llvm/IR/GlobalVariable.h
> >>  include/llvm/IR/Metadata.def
> >>  lib/Analysis/ModuleDebugInfoPrinter.cpp
> >>  lib/AsmParser/LLParser.cpp
> >>  lib/Bitcode/Reader/BitcodeReader.cpp
> >>  lib/Bitcode/Writer/BitcodeWriter.cpp
> >>  lib/CodeGen/AsmPrinter/CodeViewDebug.cpp
> >>  lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
> >>  lib/CodeGen/AsmPrinter/DwarfCompileUnit.h
> >>  lib/CodeGen/AsmPrinter/DwarfDebug.cpp
> >>  lib/IR/AsmWriter.cpp
> >>  lib/IR/DIBuilder.cpp
> >>  lib/IR/DebugInfo.cpp
> >>  lib/IR/DebugInfoMetadata.cpp
> >>  lib/IR/LLVMContextImpl.h
> >>  lib/IR/Metadata.cpp
> >>  lib/IR/Verifier.cpp
> >>  lib/Transforms/IPO/StripSymbols.cpp
> >>  lib/Transforms/Instrumentation/AddressSanitizer.cpp
> >>  test/Assembler/diglobalvariable.ll
> >>  test/Assembler/diglobalvariableexpression.ll
> >>  test/Bitcode/DIGlobalVariableExpr.ll
> >>  test/Bitcode/DIGlobalVariableExpr.ll.bc
> >>  test/Bitcode/diglobalvariable-3.8.ll
> >>  test/Bitcode/diglobalvariable-3.8.ll.bc
> >>  test/DebugInfo/X86/multiple-at-const-val.ll
> >>  test/DebugInfo/X86/pr12831.ll
> >>  test/DebugInfo/X86/split-global.ll
> >>  test/DebugInfo/X86/stack-value-dwarf4.ll
> >>  test/DebugInfo/X86/unattached-global.ll
> >>  test/Transforms/GlobalMerge/debug-info.ll
> >>  unittests/IR/MetadataTest.cpp
> >>
> >
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161118/b1dbd8f8/attachment.html>