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

[David Blaikie via llvm-commits]

On Fri, Nov 18, 2016 at 10:11 AM Adrian Prantl <aprantl at apple.com> wrote:

>
> > On Nov 18, 2016, at 9:55 AM, Duncan P. N. Exon Smith <
> 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?))

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> 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>
> 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
> >>
> >> 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/df582a1c/attachment.html>