[PATCH] D14687: Macro support in LLVM IR

Thu Nov 19 20:10:35 PST 2015

> On 2015-Nov-19, at 18:25, Eric Christopher <echristo at gmail.com> wrote:
> 
> 
> 
> On Wed, Nov 18, 2015 at 3:57 PM David Blaikie via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> On Wed, Nov 18, 2015 at 3:52 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:
> Sorry for my delay here, I lost it in my inbox.
> 
> > On 2015-Nov-17, at 09:01, David Blaikie via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> >
> >
> >
> > On Tue, Nov 17, 2015 at 1:13 AM, Amjad Aboud via llvm-commits <llvm-commits at lists.llvm.org> wrote:
> > aaboud added a comment.
> >
> > > > If I understood you correctly, you are referring to IR documentation that I
> >
> > >  forgot to update according to these changes, right?
> >
> > >  >
> >
> > >
> >
> > > Yes :).
> >
> >
> > [AA]  OK I will do that when I upload a new patch.
> 
> Thanks!
> 
> > > I'm not speaking about this from the DWARF angle, I really know almost
> >
> > >  nothing about DWARF.  The IR is used to generate DWARF, but it doesn't
> >
> > >  actually match the DWARF (from what I understand).  We need to/have the
> >
> > >  opportunity to figure things out on an as-needed-basis in the backend.
> >
> > >
> >
> > > The problem with referring to something from the compile unit is that even
> >
> > >  when the relevant code gets optimized out, we'll still have references to the
> >
> > >  full debug info graph.  This makes bugpoint reduction painful (the testcases
> >
> > >  aren't actually reduced), causes scalability problems for LTO, and bloats the
> >
> > >  debug info with descriptions that the end-user has no need for.
> >
> > >
> >
> > > IMO, references from the IR should keep alive the actual macros that are
> >
> > >  relevant in the emitted object code (after optimizations), and the rest should
> >
> > >  just be dropped.  This is the direction we're generally going with
> >
> > >  subprograms (hoping to remove the subprograms array from the compile
> >
> > >  unit soon).
> >
> > >
> >
> > > However, I frankly don't understand the whole picture here, since you didn't
> >
> > >  attach any changes to CFE for where macros get inserted into the code.
> >
> >
> > [AA] According to my understanding of macros, the compiler should pass them to the debug info as is.
> >
> > Yep,  just to re-iterate/provide some context for Duncan: This debug info is to describe macro definitions, not their use. So in codebases that are macro heavy (like GCC) you can use those macros when evaluating expressions in your debugger (since in codebases like that it's impractical to interact with many data structures without the macros).
> >
> > That is why they look the same all the way from the preprocessor till they are emitted in the object file.
> > Optimizations will change nothing in the macro debug info, and it is up to other debug info (such as line table) to hide or expose unreachable macros from the user.
> > In other words, you cannot optimize macros.
> > Also, macro debug info is related to the compile unit, so when we link two compile units, each will have its own macro debug info.
> > In DWARF5 there is a way to share encoding of macros between compile unites, but still each will have its own.
> 
> Thanks for the clarification David, sorry Amjad for being so clueless
> about this.
> 
> I'm concerned about the memory usage in LTO if every compile unit has
> included the same headers, but I guess we can can cross that bridge
> when we come to it, maybe disabling macro definitions when -flto, or
> some such?
> 
> Yeah, I'm not sure what to do there either - in the curretn design (CUs owning the macros) the macros themselves would be shared, right, so it wouldn't be a memory issue for the IR, but could be when we go to generate debug info and create the same debug info over and over again (but perhaps by the time we care we'll have been doing incremental dwarf output and it won't be such an issue? and/or we don't need to build an internal representation of macros beyond the IR anyway, so it should already be incremental?) 
> 
> I don't know what the DWARF output itself should look like if we wanted to deduplicate macro info across CUs... 
>  
> 
> As, I think you, said there's going to be some support in dwarf5 for being able to do this - it's really insufficient to do the amount of de-duplication we see right now with types though I think. We'll probably want to explore it a bit and see if there's anything we can do in a future release.
> 
> Right now I think we should, in general, not put macro information in our output unless we're specifically asked. (It's some large N on -gN iirc for gcc).

SGTM.

> 
> -eric
>  
> 
> >
> > It would be great if somebody, who know better than me the macro debug info definition, can give his opinion on this point.
> >
> > > Ah, it doesn't use DW_TAG.  Should we move DINode to DITagNode and
> >
> > >  then create a common base class called DINode?
> >
> >
> > [AA] I thought that MDNode is the base class, why should we have another base class?
> > Notice, that I am not doing something new to LLVM debug info, we already have the DILocation and DIExpression classes that inherit from MDNode and not DINode.
> > So, why cannot we have also the DIMacroNode that inherits from MDNode and has nothing to do with DINode?
> 
> I'll have another look at the patch as is once you the LangRef
> changes; I may have been wrong here.
> 
> I do think `DINode` should maybe be renamed to `DITaggedNode`, but
> that does seem perpendicular to your patch.
> 
> >
> > Repository:
> >   rL LLVM
> >
> > http://reviews.llvm.org/D14687
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits