[PATCH] D14687: Macro support in LLVM IR

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 16 10:45:51 PST 2015


> On 2015-Nov-16, at 03:52, Amjad Aboud <amjad.aboud at intel.com> wrote:
> 
> aaboud added a comment.
> 
> In http://reviews.llvm.org/D14687#289729, @dexonsmith wrote:
> 
>> This seems to be missing any changes to LangRef.
> 
> 
> If I understood you correctly, you are referring to IR documentation that I forgot to update according to these changes, right?
> 

Yes :).

>> Do we really need to add a new field to DICompileUnit?  Why would the compile unit need to reference all the macros?
> 
> 
> If not in the DICompileUnit then where do you suggest the macro tree be referred from? its own named metadata?
> According to dwarf, macro in ".debug_macinfo" section is referenced from compile-unit DIE (DW_TAG_compile_unit) using the DW_AT_macro_info attribute.

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.

>> I don't understand why you need `MacroNode` separate from `DIMacro`.  `DIMacro` seems to be the only subclass, so why split it?  Also, is `MacroNode` meant for something other than debug info?  If it's for debug info, it should have a `DI`-prefix.  It should also inherit from `DINode`.
> 
> At first I wanted to use DINode as the subclass for both DIMacro and DIMacroFile, however, DINode assumes a DW_TAG_* enumerator while the macro is using different enumerator  DW_MACINFO_*. For the same reason, the MacroNode cannot inherit from DINode.
> Also, DIMacro cannot be the base class as it is different than DIMacroFile, and both needs a base class.
> Regarding the name of MacroNode, I am fine with any name you pick, does DIMacroNode sound better?

Ah, it doesn't use DW_TAG.  Should we move DINode to DITagNode and then
create a common base class called DINode?


More information about the llvm-commits mailing list