[PATCH] D16077: DIBuilder support DI Macro creation

[Amjad Aboud via llvm-commits]

aaboud added a comment.

In https://reviews.llvm.org/D16077#498128, @aprantl wrote:

> Yes, I think putting it on DIMacroFile might be a better interface, but I'm not sure.
>  But then there is also another issue: I'm not sure whether it is legal to modify a uniquable (non-distinct) DINode because it will break uniquing. So I think DIMacroFiles must be created distinct to avoid this problem. Would this cause an issue during LTO with duplication or are they expected to be mostly unique across compile units anyway?


After short investigation there is an issue with both options.

1. Defining the Macro Dies as distinct will work from functional aspect, however, it will prevent sharing similar Macro Dies, which will be a real issue in dwarf5, where we can make usage of such sharing.
2. Defining the Macro Dies as unique will cause a functional issue, though not with the LTO, but with direct compilation like in the following example:

t.h

  #ifdef A
  #define Y X
  #else
  #define Y Z
  #endif

t1.h

  #define A
  #include "t.h"
  #undef A

t2.h

  #define B
  #include "t.h"
  #undef B

t.c

  #include "t1.h"
  #include "t2.h"

In this case the LLVM IR for "t.h" will look like this:

  !306 = !DIMacroFile(line: 2, file: !307, nodes: !308)
  !307 = !DIFile(filename: "t.h", directory: "/")
  !308 = !{!309, !310}
  !309 = !DIMacro(type: DW_MACINFO_define, line: 2, name: "Y", value: "X")
  !310 = !DIMacro(type: DW_MACINFO_define, line: 4, name: "Y", value: "Z")

Even though in this case the include of "t.h" from "t1.h" does not match the include of "t.h" from "t2.h", but because when we created the DIMacroFile we created it with empty nodes, there was a match (same macro tag, same line number, same file name, same empty node list).
The reason why LTO does not have the same problem, is that when we link the IR, the DIMacroFile already have its node list initialized, so the compare will be accurate.

So, if you ask me, I still prefer to use unique on Macro nodes, however we need to create them with the nodes and prevent changing them later.
Need to work on fixing this on the Clang part at https://reviews.llvm.org/D16135.


https://reviews.llvm.org/D16077