<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Fri, Dec 9, 2016 at 2:13 PM Adrian Prantl <<a href="mailto:aprantl@apple.com">aprantl@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br class="gmail_msg">
> On Dec 9, 2016, at 2:08 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="gmail_msg" target="_blank">dblaikie@gmail.com</a>> wrote:<br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
> On Fri, Dec 9, 2016 at 2:01 PM Adrian Prantl via Phabricator <<a href="mailto:reviews@reviews.llvm.org" class="gmail_msg" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br class="gmail_msg">
> aprantl marked 8 inline comments as done.<br class="gmail_msg">
> aprantl added inline comments.<br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
> ================<br class="gmail_msg">
> Comment at: include/llvm/IR/GlobalVariable.h:177<br class="gmail_msg">
> + using DIGlobalVarExpr =<br class="gmail_msg">
> + PointerUnion<DIGlobalVariable *, DIGlobalVariableExpression *>;<br class="gmail_msg">
> + /// Attach a DIGlobalVariable or DIGlobalVariableExpression.<br class="gmail_msg">
> ----------------<br class="gmail_msg">
> dblaikie wrote:<br class="gmail_msg">
> > 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.<br class="gmail_msg">
> 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).<br class="gmail_msg">
><br class="gmail_msg">
> 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?<br class="gmail_msg">
<br class="gmail_msg">
Maybe we are talking about different things. The PointerUnion allows us to describe a global variable that doesn't need a DIExpression like so:<br class="gmail_msg">
<br class="gmail_msg">
@g = global i32 0, !dbg !0<br class="gmail_msg">
!0 = DIGlobalVariable(name: "g", ...)<br class="gmail_msg">
<br class="gmail_msg">
without it, we'd have to describe it like so:<br class="gmail_msg">
<br class="gmail_msg">
@g = global i32 0, !dbg !0<br class="gmail_msg">
!0 = DIGlobalVariableExpression(var: !1, expr: null)<br class="gmail_msg">
!1 = DIGlobalVariable(name: "g", ...)<br class="gmail_msg">
<br class="gmail_msg">
And I think this decision is unaffected by how we handle global variables in optimizations. Let me know if I'm missing the point.<br class="gmail_msg"></blockquote><div><br></div><div>Sort of. My answer wasn't really satisfactory/accurate - your explanation refreshed my understanding so I'll make a stronger statement:<br><br>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.<br><br>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). Is it worth optimizing for? We don't do that for local variables, right?<br><br>- Dave</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br class="gmail_msg">
-- adrian<br class="gmail_msg">
<br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
> ================<br class="gmail_msg">
> Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:2209<br class="gmail_msg">
> }<br class="gmail_msg">
> + // FIXME: emitDebugInfoForGlobal() doesn't handle DIExpressions.<br class="gmail_msg">
> emitDebugInfoForGlobal(G, GV, Asm->getSymbol(GV));<br class="gmail_msg">
> ----------------<br class="gmail_msg">
> dblaikie wrote:<br class="gmail_msg">
> > Not quite sure what this means/what the implications are?<br class="gmail_msg">
> It's just something I noticed while updating the code. Could be a separate commit.<br class="gmail_msg">
><br class="gmail_msg">
> Might be good - just to discuss.<br class="gmail_msg">
><br class="gmail_msg">
> ================<br class="gmail_msg">
> Comment at: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:191<br class="gmail_msg">
> + if (Loc) {<br class="gmail_msg">
> + DwarfExpr->finalize();<br class="gmail_msg">
> addBlock(*VariableDIE, dwarf::DW_AT_location, Loc);<br class="gmail_msg">
> ----------------<br class="gmail_msg">
> dblaikie wrote:<br class="gmail_msg">
> > Should finalize return the DIELoc instead of having it kept as a separate/parallel variable that DIEDwarfExpression is updating?<br class="gmail_msg">
> Not sure I understand. You mean like<br class="gmail_msg">
> ```<br class="gmail_msg">
> class DIEDwarfExpression {<br class="gmail_msg">
> ...<br class="gmail_msg">
> DIELoc finalize() {<br class="gmail_msg">
> DwarfExpression::finalize();<br class="gmail_msg">
> return DIE;<br class="gmail_msg">
> }<br class="gmail_msg">
> }<br class="gmail_msg">
><br class="gmail_msg">
> ...<br class="gmail_msg">
> addBlock(*VariableDIE, dwarf::DW_AT_location, DwarfExpr.finalize());<br class="gmail_msg">
><br class="gmail_msg">
> Yep, something like that. (presumably a DILoc*, but yeah - that idea)<br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div></div>