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

[Adrian Prantl via llvm-commits]

> On Dec 9, 2016, at 3:08 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Fri, Dec 9, 2016 at 2:57 PM Adrian Prantl <aprantl at apple.com <mailto:aprantl at apple.com>> wrote:
> 
> > On Dec 9, 2016, at 2:41 PM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
> >
> >
> >
> > On Fri, Dec 9, 2016 at 2:13 PM Adrian Prantl <aprantl at apple.com <mailto:aprantl at apple.com>> wrote:
> >>
> >> > On Dec 9, 2016, at 2:08 PM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
> >> > dblaikie wrote:
> >> > > Is this union for backwards compatibility? (so things can still refer directly to a DIGlobalVariable?) If so, might be nice to push that compatibility earlier - map old debug info by creating new DIGlobalVariableExpressions, rather than letting the old semantics remain further through the system.
> >> > The union (the canonical definition is in DebugInfoMetadata.h exists so we don't need to waste a DIGlobalVariableExpression node on global variables that don't need an DIExpression. This is an optimization that Duncan suggested (though I can't find where at the moment).
> >> >
> >> > I assume the plan is to treat global variables the same way we treat functions - drop them if we no longer have a definition. (potentially keep a list of any that get collapsed to a constant/no asociation from an llvm::GlobalValue - but, say, if we did LTO and just optimized away a global variable because it was unused, we'd drop it the same as we drop functions). So it seems strange to me to optimize that case that I think we don't seem to be/intend to care about/have happen much/at all?
> >>
> >> Maybe we are talking about different things. The PointerUnion allows us to describe a global variable that doesn't need a DIExpression like so:
> >>
> >>   @g = global i32 0, !dbg !0
> >>   !0 = DIGlobalVariable(name: "g", ...)
> >>
> >> without it, we'd have to describe it like so:
> >>
> >>   @g = global i32 0, !dbg !0
> >>   !0 = DIGlobalVariableExpression(var: !1, expr: null)
> >>   !1 = DIGlobalVariable(name: "g", ...)
> >>
> >> And I think this decision is unaffected by how we handle global variables in optimizations. Let me know if I'm missing the point.
> >
> > Sort of. My answer wasn't really satisfactory/accurate - your explanation refreshed my understanding so I'll make a stronger statement:
> >
> > We don't have infrastructure for emitting function declarations (except as class members), and I would expect global variables to be treated the same. Which is to say I'm not sure we need infrastructure for global variables that have no value.
> 
> When a global variable is a constant, the Value that is the constant in the IR and the GlobalObject that the debug info is attached to may get optimized away, but we still can and want to represent the constant source variable in the debug information. 
> 
> Right - in that case the 'globals' list would contain the DIGlobalVariableExpression for the constant value for the DIGlobalVariable, I would imagine. (essentially the 'globals' list acts as the llvm::Global's !dbg attachment when there is no llvm::Global to use (so, for constant values, constant pieces, etc))

Yes that's the idea.

>  
> This is why the list of DIGlobalVariableExpressions in the CU remains useful.
> 
> Besides that there's also Duncan's argument from PR 31013:
>   "I argue for DIGlobalVariableExpression for layering/philosophical reasons:
>   - DIGlobalVariable describes (should describe) the source code.
>   - The use of DIGlobalVariable describes (should describe) how that maps to the object code."
> >
> > Ah, I see - you mean the case where it does have a location, it's just not described by a complex expression (it's just the address itself).
> 
> Yes.
> 
> > Is it worth optimizing for? We don't do that for local variables, right?
> 
> Under the assumption that most global variables don't have an expression, yes. Under the assumption that most well-written programs have only a handful of global variables, perhaps no.
> 
> Given the weird variations we've had in the debug info metadata in the past (for some reason the whole "extra level of indirection on the lists hanging off the CU" etc - which were, so far as I know, entirely spurious (but perhaps had some historic significance), so not an exact analogy for this) I'd prefer we have some data to backup the complexity before adding it.
> 
> (reminds me of the complexity we're carrying for ref_addr you mentioned saved half a % or something - which, I agree, is not nothing, but still seems a bit marginal)

I can't say I'm particularly fond of the contortions necessary to support the PointerUnion in the code.
Duncan — do you care to argue in favor of it as opposed to just always requiring a DIGlobalVariableExpression indirection?

-- adrian
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161209/363d0db0/attachment.html>