[WIP] DebugInfo: New metadata representation for global variables.

[pcc]

Summary

This patch reverses the edge from DIGlobalVariable to GlobalVariable. In
order to make this work I also needed to implement the following extensions
for global attached metadata:

• I implemented a capability for globals to hold multiple metadata nodes with the same kind.
• I added a byte offset field to the global attached metadata list entries. The byte offset can be used to describe the position in the global that the metadata pertains to. Passes such as merge globals can copy metadata from one global to another, adjusting the offset if necessary. This field is intended to be generically useful, for example I plan to use it to hold the offsets currently used by "bitset" metadata in the vtable opt and CFI passes.

This isn't fully ready yet, but I wanted to gauge support for the new
representation before implementing the remaining pieces.

TODO:

• Add bitcode read/write and upgrade support for the new constructs
• Use a DIExpression to represent constants
• Update ASan to copy metadata in the same way as I updated merge globals
• Update clang

Depends on D20074

[pcc]

upgrade.go1 KBDownload

I have attached the Go program that I used to upgrade most of the test cases.

[aprantl]

Instead of baking a byte-offset into the IR, I was more thinking of using the DIExpression to describe the offset:

For example, to describe a global at offset 4, we would use a DIExpression(DW_OP_constu 4, DW_OP_plus) to adjust the offset.

test/CodeGen/ARM/2011-08-02-MergedGlobalDbg.ll

1

Just a general remark: This should really be split into one test that runs opt -arm-global-merge, and one that runs llc and checks the DWARF output.

[aprantl]

If we agree that using the DIExpression is how we want to proceed, it might be easier to stage this by introducing a DIExpression to DIGlobalVariable first in a separate commit.

[pcc]

Instead of baking a byte-offset into the IR, I was more thinking of using the DIExpression to describe the offset:

I suppose we could do that, but I reckon that would make things more complicated:

• each constant would need two DIExpressions, one for the case where a global exists and an address can be derived from the attached GlobalVariable's address, and one for the case where the global has been optimized away (where we'd use DW_OP_stack_value to describe the constant)
• it would add complexity to the copyMetadata function, as it would need to be able to manipulate and rewrite DIExpressions
• I also plan to use the offset in other kinds of metadata attached to globals, such as the type metadata for CFI and vtable opt. If we use a separate representation for each kind of metadata, copyMetadata would need to know how to adjust the offset in each kind of metadata

[aprantl]

In D20147#427453, @pcc wrote:

Instead of baking a byte-offset into the IR, I was more thinking of using the DIExpression to describe the offset:

I suppose we could do that, but I reckon that would make things more complicated:

• each constant would need two DIExpressions, one for the case where a global exists and an address can be derived from the attached GlobalVariable's address, and one for the case where the global has been optimized away (where we'd use DW_OP_stack_value to describe the constant)

Not sure if I follow. While the constant still exists as a global in the IR, the DIExpression would be empty.
Whenever a constant is deleted from IR, we invoke a utility that

• adds the DIGlobalVariable to the DICompileUnit to retain it
• translates the constexpr into a DIExpression

This is assuming that after GlobalMerge happened no merged globals will be removed.

• it would add complexity to the copyMetadata function, as it would need to be able to manipulate and rewrite DIExpressions

Can you give a quick example for this?

• I also plan to use the offset in other kinds of metadata attached to globals, such as the type metadata for CFI and vtable opt. If we use a separate representation for each kind of metadata, copyMetadata would need to know how to adjust the offset in each kind of metadata

[pcc]

In D20147#427470, @aprantl wrote:

In D20147#427453, @pcc wrote:

Instead of baking a byte-offset into the IR, I was more thinking of using the DIExpression to describe the offset:

I suppose we could do that, but I reckon that would make things more complicated:

• each constant would need two DIExpressions, one for the case where a global exists and an address can be derived from the attached GlobalVariable's address, and one for the case where the global has been optimized away (where we'd use DW_OP_stack_value to describe the constant)

Not sure if I follow. While the constant still exists as a global in the IR, the DIExpression would be empty.
Whenever a constant is deleted from IR, we invoke a utility that

• adds the DIGlobalVariable to the DICompileUnit to retain it

I'm not changing the relationship between DICompileUnit and DIGlobalVariable in this patch. This means that if we change the DIExpression, we'd most likely need to find the DIGlobalVariable in the DICompileUnit's global list and rewrite it. But I suppose that's fine if it's temporary.

• translates the constexpr into a DIExpression

This is assuming that after GlobalMerge happened no merged globals will be removed.

Okay. So if I follow you, the interpretation of the DIExpression will change depending on whether the DIGlobalVariable was attached to a GlobalVariable. If it is attached, we'd start with the address of the GlobalVariable on the stack, and if it isn't attached we'd start with an empty stack. Is that what you had in mind?

• it would add complexity to the copyMetadata function, as it would need to be able to manipulate and rewrite DIExpressions

Can you give a quick example for this?

Well, okay, I suppose that if we're just adding an offset it wouldn't be too hard to append (DW_OP_constu, X, DW_OP_plus) to the DIExpression. But we'd (yes, temporarily) need to rewrite the DICompileUnit's global list, and each new kind of metadata node that we attach to globals would require its own handling.

[aprantl]

In D20147#427547, @pcc wrote:

In D20147#427470, @aprantl wrote:

In D20147#427453, @pcc wrote:

Instead of baking a byte-offset into the IR, I was more thinking of using the DIExpression to describe the offset:

I suppose we could do that, but I reckon that would make things more complicated:

• each constant would need two DIExpressions, one for the case where a global exists and an address can be derived from the attached GlobalVariable's address, and one for the case where the global has been optimized away (where we'd use DW_OP_stack_value to describe the constant)

Not sure if I follow. While the constant still exists as a global in the IR, the DIExpression would be empty.
Whenever a constant is deleted from IR, we invoke a utility that

• adds the DIGlobalVariable to the DICompileUnit to retain it

I'm not changing the relationship between DICompileUnit and DIGlobalVariable in this patch. This means that if we change the DIExpression, we'd most likely need to find the DIGlobalVariable in the DICompileUnit's global list and rewrite it. But I suppose that's fine if it's temporary.

Agreed, that sounds not so great. Would it be possible to change the relationship before landing this patch?

• translates the constexpr into a DIExpression

This is assuming that after GlobalMerge happened no merged globals will be removed.

Okay. So if I follow you, the interpretation of the DIExpression will change depending on whether the DIGlobalVariable was attached to a GlobalVariable. If it is attached, we'd start with the address of the GlobalVariable on the stack, and if it isn't attached we'd start with an empty stack. Is that what you had in mind?

The DIExpressions in LLVM IR have an implicit first element that is the "base" location of the entity that is being described. The DWARF backend can make the following decision:

• if the DIGlobalVariable is found via a !dbg attachment on a global, the address of the global is emitted as the first element of the dwarf expression, followed by the DIExpression,
• if the DIExpression describes a constant (i.e., it ends with DW_OP_stack_value, DW_OP_implicit_value, DW_OP_implicit_pointer, ...) the DIExpression is emitted as is.

• it would add complexity to the copyMetadata function, as it would need to be able to manipulate and rewrite DIExpressions

Can you give a quick example for this?

Well, okay, I suppose that if we're just adding an offset it wouldn't be too hard to append (DW_OP_constu, X, DW_OP_plus) to the DIExpression. But we'd (yes, temporarily) need to rewrite the DICompileUnit's global list, and each new kind of metadata node that we attach to globals would require its own handling.

Nitpick: I think it would need to prepend the DW_OP_constu, X, DW_OP_plus.

[pcc]

Refresh and split out dependencies

[pcc]

Okay, I've updated this per Adrian's suggestions, split out some of the dependencies (D20412, D20413, D20414) and created a clang patch (D20415). This should be ready for review now.

[pcc]

• Only emit the expr if we have a global and the expr form is not specifically recognized

[aprantl]

Thanks for putting so much effort into this, this is starting to look really great!

include/llvm/IR/DIBuilder.h

502

If we haven't already, we will need to add explicit support for DW_OP_stack_value to DwarfExpression to ensure that we don't emit when generating DWARF 2. (And yes, without it the expression will be ambiguous).

include/llvm/IR/DebugInfoMetadata.h

1828

... or is the (constant) variable.

1831

Oh yes :-)

1868

All the \brief's are redundant unless you want to include more than the first sentence in the brief description. Makes the code a bit more readable IMHO.
(I know this is just copied, but we might as well fix it up now).

lib/Bitcode/Reader/BitcodeReader.cpp

2614

I'm not sure if Expr is guaranteed to be materialized at this point. You may need to store a list of DIExpressions and fix them up later.
Duncan?

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

127

Ah, great! This should probably say "DWARF 3 and earlier",

[pcc]

• Address review comments

lib/Bitcode/Reader/BitcodeReader.cpp

2614

My understanding is that lazy materialization is only used for functions, which is why we need the fixup code for upgrading function metadata, but not for globals.

[pcc]

Ping.

[aprantl]

include/llvm/IR/DIBuilder.h

443

Maybe this should default to nullptr?

include/llvm/IR/GlobalVariable.h

32

Why Module?

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

167–171

Can you remind me why this is no longer necessary?

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

470

Nice! This should probably be a separate NFC commit.

492

if (const GlobalVariable *Global = GVMap.lookup(GV))
  CU.getOrCreateGlobalVariableDIE(GV, Global);

?

lib/CodeGen/GlobalMerge.cpp

449

Maybe add a short comment about what's happening here?

lib/IR/DebugInfoMetadata.cpp

566 ↗

(On Diff #57986)

Technically correct, but DW_OP_piece must be even farther at the end ;-)

In the DWARF 5 draft Figure D.61, there is an expression example that uses both to synthesize a struct from the constants {1, 2}:

DW_OP_lit1 DW_OP_stack_value DW_OP_piece(4)
DW_OP_lit2 DW_OP_stack_value DW_OP_piece(4)

[pcc]

• Refresh
• Address review comments
• Simplify: do not use a separate loop to create CU DIEs.

include/llvm/IR/GlobalVariable.h

32

Just a drive-by cleanup, I was re-ordering these declarations alphabetically.

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

167–171

Both of these cases are now represented using expressions: the first using DW_OP_stack_value, and the second using DW_OP_plus to apply the adjustment.

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

470

r271360

492

Yes, almost, but we still need to create DIEs for unattached globals here.

lib/IR/DebugInfoMetadata.cpp

566 ↗

(On Diff #57986)

Okay, adjusted to say what we currently support.

[aprantl]

lib/IR/DebugInfoMetadata.cpp

566 ↗

(On Diff #59168)

That's probably good enough for this patch.

[pcc]

• Simplify: Remove unnecessary temporary variable

[pcc]

We currently aren't correctly deserializing global variable metadata from lazy loaded modules, so should probably hold off until that's fixed.

[dexonsmith]

One fix for that would be to lazy-load global variables/aliases, and load the metadata block when the first global variable is accessed.

[pcc]

The fix I'm working on is to move the GLOBALVAR_ATTACHMENT record to the METADATA block. I'll send it out in a bit.

[llvm-commits]

I wonder if that's the right long term solution?

Lazy-loading GlobalVariables could eliminate the global constants block, and we could move GlobalVariable-related metadata out of the global metadata block as well.

I think lazy-loading GlobalVariables could result in important speedups for -flto=thin, and solves this problem as a side effect.

[dexonsmith]

It would also speedup llvm-ar, and other tools that use lazy-loading.

[pcc]

While it's possible that that would result in a performance improvement, it's unclear that that would be the case, and I don't think we should make such a change without evidence. For ThinLTO and llvm-ar a more effective performance improvement would be to pre-compute the symbol table (see pr27551), which would avoid the need to read most of the bitcode file. There's also the possibility that lazy loading GlobalVariables would slow things down, as the benefit of lazy loading globals would generally be less than for functions.

[mehdi_amini]

I agree with you on llvm-ar.
For ThinLTO it is totally different and the symbol table does not help: when you want to import a function, you need to lazy-load the module and materialize the functions you want to import.
This is where we want as much laziness with other globals and more importantly metadata in general.

[pcc]

• Refresh

[aprantl]

While going through my review queue I was wondering what is the state of this?

[pcc]

My recollection is that the lazy loading issues discussed are not a blocker, as they do not represent a perf regression (DIGlobalVariable metadata is already being loaded eagerly). Changing the bitcode representation for global variables to allow for lazy loading can be done separately.

If there are no other issues this patch most likely just needs a sign off (and a refresh given its age).

[mehdi_amini]

In D20147#539056, @pcc wrote:

My recollection is that the lazy loading issues discussed are not a blocker, as they do not represent a perf regression (DIGlobalVariable metadata is already being loaded eagerly). Changing the bitcode representation for global variables to allow for lazy loading can be done separately.

If there are no other issues this patch most likely just needs a sign off (and a refresh given its age).

I may misunderstand what this will change, it seems to me that this is a regression in lazy loading. Currently lazy loading bitcode does *not* load global metadata, while after this change it will load all what is reachable from the metadata attached to a global.
It would matter when the linker lazy load to perform symbol resolution for instance, which is a blocker I think.

On the other side, if something like D23132 lands, that would probably unblock this as we would have another path than lazy-loading bitcode.

[pcc]

Currently lazy loading bitcode does *not* load global metadata, while after this change it will load all what is reachable from the metadata attached to a global.

I don't think that's accurate. The metadata will continue to be stored in the global metadata block, and if the client requests lazy loading of metadata will only be loaded during materializeMetadata (alongside other metadata in the global metadata block), except that now it will be attached to the global using a METADATA_GLOBAL_DECL_ATTACHMENT record.

[mehdi_amini]

In D20147#539124, @pcc wrote:

Currently lazy loading bitcode does *not* load global metadata, while after this change it will load all what is reachable from the metadata attached to a global.

I don't think that's accurate. The metadata will continue to be stored in the global metadata block, and if the client requests lazy loading of metadata will only be loaded during materializeMetadata (alongside other metadata in the global metadata block), except that now it will be attached to the global using a METADATA_GLOBAL_DECL_ATTACHMENT record.

OK, if so, then forget anything I wrote above :)

[aprantl]

Thanks, from my end this looks great!

If you happen to have a script to upgrade the testcases, it would be great if you could file a PR for this and attach the script there and comment in the commit message. This will make it easier for people with out-of-tree testcases.

lib/Bitcode/Reader/BitcodeReader.cpp

2611

Please make sure to add a binary bitcode upgrade test case for this.

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

130

Since we already have a special createConstantValueExpression() method we could also add an Optional<uint64> getConstantValueExpression() accessor?

[pcc]

If you happen to have a script to upgrade the testcases, it would be great if you could file a PR for this and attach the script there and comment in the commit message

Filed PR30362 with an upgrade program.

lib/Bitcode/Reader/BitcodeReader.cpp

2611

After this patch test/Bitcode/diglobalvariable-3.8.ll and test/Bitcode/dityperefs-3.8.ll will respectively be testing the constant and variable cases.

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

130

Maybe if we need to query this in more than one place. For now it seems clearer to be direct here.

[aprantl]

Thanks for all the hard work!

include/llvm/IR/DIBuilder.h

public:
     /// \param Context     Variable scope.
     /// \param Name        Name of the variable.
     /// \param LinkageName Mangled  name of the variable.
     /// \param File        File where this variable is defined.
     /// \param LineNo      Line number.
     /// \param Ty          Variable Type.
     /// \param isLocalToUnit Boolean flag indicate whether this variable is
     ///                      externally visible or not.
-    /// \param Val         llvm::Value of the variable.
+    /// \param Val         Value of the variable, if a constant.
     /// \param Decl        Reference to the corresponding declaration.
     DIGlobalVariable *createGlobalVariable(DIScope *Context, StringRef Name,
                                            StringRef LinkageName, DIFile *File,
                                            unsigned LineNo, DIType *Ty,
                                            bool isLocalToUnit,
-                                           llvm::Constant *Val,
+                                           llvm::ConstantData *Val,
                                            MDNode *Decl = nullptr);
 
     /// Identical to createGlobalVariable
     /// except that the resulting DbgNode is temporary and meant to be RAUWed.
     DIGlobalVariable *createTempGlobalVariableFwdDecl(
         DIScope *Context, StringRef Name, StringRef LinkageName, DIFile *File,
-        unsigned LineNo, DIType *Ty, bool isLocalToUnit, llvm::Constant *Val,
-        MDNode *Decl = nullptr);
+        unsigned LineNo, DIType *Ty, bool isLocalToUnit,
+        llvm::ConstantData *Val, MDNode *Decl = nullptr);
 
     /// Create a new descriptor for an auto variable.  This is a local variable
     /// that is not a subprogram parameter.
     ///
     /// \c Scope must be a \a DILocalScope, and thus its scope chain eventually
     /// leads to a \a DISubprogram.
     ///
     /// If \c AlwaysPreserve, this variable will be referenced from its
public:
     /// \param SizeInBits   Size of the piece in bits.
     DIExpression *createBitPieceExpression(unsigned OffsetInBits,
                                            unsigned SizeInBits);
 
     /// Create a new descriptor for the specified subprogram.
     /// See comments in DISubprogram* for descriptions of these fields.
     /// \param Scope         Function scope.
     /// \param Name          Function name.
     /// \param LinkageName   Mangled function name.
     /// \param File          File where this variable is defined.
     /// \param LineNo        Line number.
     /// \param Ty            Function type.
     /// \param isLocalToUnit True if this function is not externally visible.
     /// \param isDefinition  True if this is a function definition.
     /// \param ScopeLine     Set to the beginning of the scope this starts
     /// \param Flags         e.g. is this function prototyped or not.
     ///                      These flags are used to emit dwarf attributes.

include/llvm/IR/DebugInfoMetadata.h

 //===- llvm/IR/DebugInfoMetadata.h - Debug info metadata --------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // Declarations for metadata specific to debug info.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_IR_DEBUGINFOMETADATA_H
 #define LLVM_IR_DEBUGINFOMETADATA_H
 
+#include "llvm/IR/Constants.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/Support/Dwarf.h"
 
 // Helper macros for defining get() overrides.
 #define DEFINE_MDNODE_GET_UNPACK_IMPL(...) __VA_ARGS__
 #define DEFINE_MDNODE_GET_UNPACK(ARGS) DEFINE_MDNODE_GET_UNPACK_IMPL ARGS
 #define DEFINE_MDNODE_GET_DISTINCT_TEMPORARY(CLASS, FORMAL, ARGS)              \
   static CLASS *getDistinct(LLVMContext &Context,                              \
static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == DILocalVariableKind ||
            MD->getMetadataID() == DIGlobalVariableKind;
   }
 };
 
 /// \brief Global variables.
 ///
 /// TODO: Remove DisplayName.  It's always equal to Name.
 class DIGlobalVariable : public DIVariable {
   friend class LLVMContextImpl;
   friend class MDNode;
 
   bool IsLocalToUnit;
   bool IsDefinition;
 
   DIGlobalVariable(LLVMContext &C, StorageType Storage, unsigned Line,
                    bool IsLocalToUnit, bool IsDefinition,
                    ArrayRef<Metadata *> Ops)
       : DIVariable(C, DIGlobalVariableKind, Storage, Line, Ops),
         IsLocalToUnit(IsLocalToUnit), IsDefinition(IsDefinition) {}
   ~DIGlobalVariable() = default;
 
   static DIGlobalVariable *
   getImpl(LLVMContext &Context, DIScope *Scope, StringRef Name,
           StringRef LinkageName, DIFile *File, unsigned Line, DITypeRef Type,
-          bool IsLocalToUnit, bool IsDefinition, Constant *Variable,
-          DIDerivedType *StaticDataMemberDeclaration, StorageType Storage,
-          bool ShouldCreate = true) {
+          bool IsLocalToUnit, bool IsDefinition,
+          ConstantData *ConstantValue, DIDerivedType *StaticDataMemberDeclaration,
+          StorageType Storage, bool ShouldCreate = true) {
     return getImpl(Context, Scope, getCanonicalMDString(Context, Name),
                    getCanonicalMDString(Context, LinkageName), File, Line, Type,
                    IsLocalToUnit, IsDefinition,
-                   Variable ? ConstantAsMetadata::get(Variable) : nullptr,
+                   ConstantValue ? ConstantAsMetadata::get(ConstantValue)
+                                 : nullptr,
                    StaticDataMemberDeclaration, Storage, ShouldCreate);
   }
   static DIGlobalVariable *
   getImpl(LLVMContext &Context, Metadata *Scope, MDString *Name,
           MDString *LinkageName, Metadata *File, unsigned Line, Metadata *Type,
-          bool IsLocalToUnit, bool IsDefinition, Metadata *Variable,
+          bool IsLocalToUnit, bool IsDefinition, Metadata *ConstantValue,
           Metadata *StaticDataMemberDeclaration, StorageType Storage,
           bool ShouldCreate = true);
 
   TempDIGlobalVariable cloneImpl() const {
     return getTemporary(getContext(), getScope(), getName(), getLinkageName(),
                         getFile(), getLine(), getType(), isLocalToUnit(),
-                        isDefinition(), getVariable(),
+                        isDefinition(), getConstantValue(),
                         getStaticDataMemberDeclaration());
   }
 
 public:
   DEFINE_MDNODE_GET(DIGlobalVariable,
                     (DIScope * Scope, StringRef Name, StringRef LinkageName,
                      DIFile *File, unsigned Line, DITypeRef Type,
-                     bool IsLocalToUnit, bool IsDefinition, Constant *Variable,
+                     bool IsLocalToUnit, bool IsDefinition,
+                     ConstantData *ConstantValue,
                      DIDerivedType *StaticDataMemberDeclaration),
                     (Scope, Name, LinkageName, File, Line, Type, IsLocalToUnit,
-                     IsDefinition, Variable, StaticDataMemberDeclaration))
+                     IsDefinition, ConstantValue, StaticDataMemberDeclaration))
   DEFINE_MDNODE_GET(DIGlobalVariable,
                     (Metadata * Scope, MDString *Name, MDString *LinkageName,
                      Metadata *File, unsigned Line, Metadata *Type,
-                     bool IsLocalToUnit, bool IsDefinition, Metadata *Variable,
+                     bool IsLocalToUnit, bool IsDefinition,
+                     Metadata *ConstantValue,
                      Metadata *StaticDataMemberDeclaration),
                     (Scope, Name, LinkageName, File, Line, Type, IsLocalToUnit,
-                     IsDefinition, Variable, StaticDataMemberDeclaration))
+                     IsDefinition, ConstantValue, StaticDataMemberDeclaration))
 
   TempDIGlobalVariable clone() const { return cloneImpl(); }
 
   bool isLocalToUnit() const { return IsLocalToUnit; }
   bool isDefinition() const { return IsDefinition; }
   StringRef getDisplayName() const { return getStringOperand(4); }
   StringRef getLinkageName() const { return getStringOperand(5); }
-  Constant *getVariable() const {
-    if (auto *C = cast_or_null<ConstantAsMetadata>(getRawVariable()))
-      return dyn_cast<Constant>(C->getValue());
+  ConstantData *getConstantValue() const {
+    if (auto *C = cast_or_null<ConstantAsMetadata>(getRawConstantValue()))
+      return dyn_cast<ConstantData>(C->getValue());
     return nullptr;
   }
   DIDerivedType *getStaticDataMemberDeclaration() const {
     return cast_or_null<DIDerivedType>(getRawStaticDataMemberDeclaration());
   }
 
   MDString *getRawLinkageName() const { return getOperandAs<MDString>(5); }
-  Metadata *getRawVariable() const { return getOperand(6); }
+  Metadata *getRawConstantValue() const { return getOperand(6); }
   Metadata *getRawStaticDataMemberDeclaration() const { return getOperand(7); }
 
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == DIGlobalVariableKind;
   }
 };
 
 /// \brief Local variable.

include/llvm/IR/GlobalVariable.h

 
 #include "llvm/ADT/Twine.h"
 #include "llvm/ADT/ilist_node.h"
 #include "llvm/IR/GlobalObject.h"
 #include "llvm/IR/OperandTraits.h"
 
 namespace llvm {
 
-class Module;
 class Constant;
+class DIGlobalVariable;
+class Module;
 template <typename ValueSubClass> class SymbolTableListTraits;
 
 class GlobalVariable : public GlobalObject, public ilist_node<GlobalVariable> {
   friend class SymbolTableListTraits<GlobalVariable>;
   void *operator new(size_t, unsigned) = delete;
   void operator=(const GlobalVariable &) = delete;
   GlobalVariable(const GlobalVariable &) = delete;
 
public:
   /// and deletes it.
   ///
   void eraseFromParent() override;
 
   /// Drop all references in preparation to destroy the GlobalVariable. This
   /// drops not only the reference to the initializer but also to any metadata.
   void dropAllReferences();
 
+  void addDebugInfo(DIGlobalVariable *GV);
+  void getDebugInfo(
+      SmallVectorImpl<std::pair<unsigned, DIGlobalVariable *>> &GVs) const;
+
   // Methods for support type inquiry through isa, cast, and dyn_cast:
   static inline bool classof(const Value *V) {
     return V->getValueID() == Value::GlobalVariableVal;
   }
 };
 
 template <>
 struct OperandTraits<GlobalVariable> :
   public OptionalOperandTraits<GlobalVariable> {
 };
 
 DEFINE_TRANSPARENT_OPERAND_ACCESSORS(GlobalVariable, Value)
 
 } // End llvm namespace
 
 #endif

lib/Bitcode/Reader/BitcodeReader.cpp

case bitc::METADATA_GLOBAL_VAR: {
           GET_OR_DISTINCT(DIGlobalVariable,
                           (Context, getMDOrNull(Record[1]),
                            getMDString(Record[2]), getMDString(Record[3]),
                            getMDOrNull(Record[4]), Record[5],
                            getDITypeRefOrNull(Record[6]), Record[7], Record[8],
                            getMDOrNull(Record[9]), getMDOrNull(Record[10]))),
           NextMetadataNo++);
       break;
     }
     case bitc::METADATA_LOCAL_VAR: {
       // 10th field is for the obseleted 'inlinedAt:' field.
       if (Record.size() < 8 || Record.size() > 10)
         return error("Invalid record");
 
       // 2nd field used to be an artificial tag, either DW_TAG_auto_variable or
       // DW_TAG_arg_variable.
       IsDistinct = Record[0];
       bool HasTag = Record.size() > 8;
       MetadataList.assignValue(
           GET_OR_DISTINCT(DILocalVariable,
std::error_code BitcodeReader::parseGlobalObjectAttachment(
     GlobalObject &GO, ArrayRef<uint64_t> Record) {
   for (unsigned I = 0, E = Record.size(); I != E; I += 2) {
     auto K = MDKindMap.find(Record[I]);
     if (K == MDKindMap.end())
       return error("Invalid ID");
     MDNode *MD = MetadataList.getMDNodeFwdRefOrNull(Record[I + 1]);
     if (!MD)
       return error("Invalid metadata attachment");
-    GO.setMetadata(K->second, MD);
+    GO.addMetadata(K->second, 0, MD);
   }
   return std::error_code();
 }
 
 /// Parse metadata attachments.
 std::error_code BitcodeReader::parseMetadataAttachment(Function &F) {
   if (Stream.EnterSubBlock(bitc::METADATA_ATTACHMENT_ID))
     return error("Invalid record");

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

unsigned DwarfCompileUnit::getOrCreateSourceID(StringRef FileName,
 
   // FIXME: add a better feature test than hasRawTextSupport. Even better,
   // extend .file to support this.
   return Asm->OutStreamer->EmitDwarfFileDirective(
       0, DirName, FileName,
       Asm->OutStreamer->hasRawTextSupport() ? 0 : getUniqueID());
 }
 
-// Return const expression if value is a GEP to access merged global
-// constant. e.g.
-// i8* getelementptr ({ i8, i8, i8, i8 }* @_MergedGlobals, i32 0, i32 0)
-static const ConstantExpr *getMergedGlobalExpr(const Value *V) {
-  const ConstantExpr *CE = dyn_cast_or_null<ConstantExpr>(V);
-  if (!CE || CE->getNumOperands() != 3 ||
-      CE->getOpcode() != Instruction::GetElementPtr)
-    return nullptr;
-
-  // First operand points to a global struct.
-  Value *Ptr = CE->getOperand(0);
-  GlobalValue *GV = dyn_cast<GlobalValue>(Ptr);
-  if (!GV || !isa<StructType>(GV->getValueType()))
-    return nullptr;
-
-  // Second operand is zero.
-  const ConstantInt *CI = dyn_cast_or_null<ConstantInt>(CE->getOperand(1));
-  if (!CI || !CI->isZero())
-    return nullptr;
-
-  // Third operand is offset.
-  if (!isa<ConstantInt>(CE->getOperand(2)))
-    return nullptr;
-
-  return CE;
-}
-
 /// getOrCreateGlobalVariableDIE - get or create global variable DIE.
 DIE *DwarfCompileUnit::getOrCreateGlobalVariableDIE(
-    const DIGlobalVariable *GV) {
+    const DIGlobalVariable *GV, const GlobalVariable *Global, unsigned Offset) {
   // Check for pre-existence.
   if (DIE *Die = getDIE(GV))
     return Die;
 
   assert(GV);
 
   auto *GVContext = GV->getScope();
   auto *GTy = DD->resolve(GV->getType());
DIE *DwarfCompileUnit::getOrCreateGlobalVariableDIE(
 
   if (!GV->isDefinition())
     addFlag(*VariableDIE, dwarf::DW_AT_declaration);
   else
     addGlobalName(GV->getName(), *VariableDIE, DeclContext);
 
   // Add location.
   bool addToAccelTable = false;
-  if (auto *Global = dyn_cast_or_null<GlobalVariable>(GV->getVariable())) {
+  if (Global) {
     addToAccelTable = true;
     DIELoc *Loc = new (DIEValueAllocator) DIELoc;
     const MCSymbol *Sym = Asm->getSymbol(Global);
     if (Global->isThreadLocal()) {
       if (Asm->TM.Options.EmulatedTLS) {
         // TODO: add debug info for emulated thread local mode.
       } else {
         // FIXME: Make this work with -gsplit-dwarf.
         unsigned PointerSize = Asm->getDataLayout().getPointerSize();
         assert((PointerSize == 4 || PointerSize == 8) &&
                "Add support for other sizes if necessary");
         // Based on GCC's support for TLS:
         if (!DD->useSplitDwarf()) {
           // 1) Start with a constNu of the appropriate pointer size
           addUInt(*Loc, dwarf::DW_FORM_data1, PointerSize == 4
if (Global->isThreadLocal()) {
         // 3) followed by an OP to make the debugger do a TLS lookup.
         addUInt(*Loc, dwarf::DW_FORM_data1,
                 DD->useGNUTLSOpcode() ? dwarf::DW_OP_GNU_push_tls_address
                                       : dwarf::DW_OP_form_tls_address);
       }
     } else {
       DD->addArangeLabel(SymbolCU(this, Sym));
       addOpAddress(*Loc, Sym);
+      if (Offset != 0) {
+        addUInt(*Loc, dwarf::DW_FORM_data1, dwarf::DW_OP_constu);
+        addUInt(*Loc, dwarf::DW_FORM_udata, Offset);
+        addUInt(*Loc, dwarf::DW_FORM_data1, dwarf::DW_OP_plus);
+      }
     }
 
     addBlock(*VariableDIE, dwarf::DW_AT_location, Loc);
     if (DD->useAllLinkageNames())
       addLinkageName(*VariableDIE, GV->getLinkageName());
   } else if (const ConstantInt *CI =
-                 dyn_cast_or_null<ConstantInt>(GV->getVariable())) {
+                 dyn_cast_or_null<ConstantInt>(GV->getConstantValue())) {
     addConstantValue(*VariableDIE, CI, GTy);
-  } else if (const ConstantExpr *CE = getMergedGlobalExpr(GV->getVariable())) {
-    addToAccelTable = true;
-    // GV is a merged global.
-    DIELoc *Loc = new (DIEValueAllocator) DIELoc;
-    auto *Ptr = cast<GlobalValue>(CE->getOperand(0));
-    MCSymbol *Sym = Asm->getSymbol(Ptr);
-    DD->addArangeLabel(SymbolCU(this, Sym));
-    addOpAddress(*Loc, Sym);
-    addUInt(*Loc, dwarf::DW_FORM_data1, dwarf::DW_OP_constu);
-    SmallVector<Value *, 3> Idx(CE->op_begin() + 1, CE->op_end());
-    addUInt(*Loc, dwarf::DW_FORM_udata,
-            Asm->getDataLayout().getIndexedOffsetInType(Ptr->getValueType(), Idx));
-    addUInt(*Loc, dwarf::DW_FORM_data1, dwarf::DW_OP_plus);
-    addBlock(*VariableDIE, dwarf::DW_AT_location, Loc);
   }
 
   if (addToAccelTable) {
     DD->addAccelName(GV->getName(), *VariableDIE);
 
     // If the linkage name is different than the name, go ahead and output
     // that as well into the name table.
     if (GV->getLinkageName() != "" && GV->getName() != GV->getLinkageName())
       DD->addAccelName(GV->getLinkageName(), *VariableDIE);
if (auto *NS = dyn_cast<DINamespace>(Entity))
     EntityDie = getOrCreateNameSpace(NS);
   else if (auto *M = dyn_cast<DIModule>(Entity))
     EntityDie = getOrCreateModule(M);
   else if (auto *SP = dyn_cast<DISubprogram>(Entity))
     EntityDie = getOrCreateSubprogramDIE(SP);
   else if (auto *T = dyn_cast<DIType>(Entity))
     EntityDie = getOrCreateTypeDIE(T);
   else if (auto *GV = dyn_cast<DIGlobalVariable>(Entity))
-    EntityDie = getOrCreateGlobalVariableDIE(GV);
+    EntityDie = getOrCreateGlobalVariableDIE(GV, nullptr, 0);
   else
     EntityDie = getDIE(Entity);
   assert(EntityDie);
   addSourceLine(*IMDie, Module->getLine(), Module->getScope()->getFilename(),
                 Module->getScope()->getDirectory());
   addDIEEntry(*IMDie, dwarf::DW_AT_import, *EntityDie);
   StringRef Name = Module->getName();
   if (!Name.empty())

lib/CodeGen/AsmPrinter/DwarfDebug.cpp

 // global DIEs and emit initial debug info sections. This is invoked by
 // the target AsmPrinter.
 void DwarfDebug::beginModule() {
   if (DisableDebugInfoPrinting)
     return;
 
   const Module *M = MMI->getModule();
 
-  unsigned NumDebugCUs = 0;
-  for (DICompileUnit *CUNode : M->debug_compile_units()) {
-    (void)CUNode;
-    ++NumDebugCUs;
-  }
-
+  unsigned NumDebugCUs = std::distance(M->debug_compile_units_begin(),
+                                       M->debug_compile_units_end());
   // Tell MMI whether we have debug info.
   MMI->setDebugInfoAvailability(NumDebugCUs > 0);
   SingleCU = NumDebugCUs == 1;
 
+  for (DICompileUnit *CUNode : M->debug_compile_units())
+    constructDwarfCompileUnit(CUNode);
+
+  std::map<DIGlobalVariable *, std::pair<const GlobalVariable *, unsigned>>
+      GVMap;
+  for (const GlobalVariable &Global : M->globals()) {
+    SmallVector<std::pair<unsigned, DIGlobalVariable *>, 1> GVs;
+    Global.getDebugInfo(GVs);
+    for (auto &GV : GVs)
+      GVMap[GV.second] = {&Global, GV.first};
+  }
+
   for (DICompileUnit *CUNode : M->debug_compile_units()) {
-    DwarfCompileUnit &CU = constructDwarfCompileUnit(CUNode);
+    DwarfCompileUnit &CU = *CUMap.lookup(CUNode);
     for (auto *IE : CUNode->getImportedEntities())
       CU.addImportedEntity(IE);
-    for (auto *GV : CUNode->getGlobalVariables())
-      CU.getOrCreateGlobalVariableDIE(GV);
+    for (auto *GV : CUNode->getGlobalVariables()) {
+      const GlobalVariable *Global = nullptr;
+      unsigned Offset = 0;
+      auto I = GVMap.find(GV);
+      if (I != GVMap.end()) {
+        Global = I->second.first;
+        Offset = I->second.second;
+      }
+      CU.getOrCreateGlobalVariableDIE(GV, Global, Offset);
+    }
     for (auto *Ty : CUNode->getEnumTypes()) {
       // The enum types array by design contains pointers to
       // MDNodes rather than DIRefs. Unique them here.
       CU.getOrCreateTypeDIE(cast<DIType>(Ty));
     }
     for (auto *Ty : CUNode->getRetainedTypes()) {
       // The retained types array by design contains pointers to
       // MDNodes rather than DIRefs. Unique them here.

lib/CodeGen/GlobalMerge.cpp

while (i != -1) {
 
     StructType *MergedTy = StructType::get(M.getContext(), Tys);
     Constant *MergedInit = ConstantStruct::get(MergedTy, Inits);
 
     GlobalVariable *MergedGV = new GlobalVariable(
         M, MergedTy, isConst, GlobalValue::PrivateLinkage, MergedInit,
         "_MergedGlobals", nullptr, GlobalVariable::NotThreadLocal, AddrSpace);
 
+    const StructLayout *MergedLayout = DL.getStructLayout(MergedTy);
+
     for (ssize_t k = i, idx = 0; k != j; k = GlobalSet.find_next(k), ++idx) {
       GlobalValue::LinkageTypes Linkage = Globals[k]->getLinkage();
       std::string Name = Globals[k]->getName();
 
+      MergedGV->copyMetadata(Globals[k], MergedLayout->getElementOffset(idx));
+
       Constant *Idx[2] = {
         ConstantInt::get(Int32Ty, 0),
         ConstantInt::get(Int32Ty, idx),
       };
       Constant *GEP =
           ConstantExpr::getInBoundsGetElementPtr(MergedTy, MergedGV, Idx);
       Globals[k]->replaceAllUsesWith(GEP);
       Globals[k]->eraseFromParent();

test/CodeGen/ARM/2011-08-02-MergedGlobalDbg.ll

 ; RUN: llc -arm-global-merge -global-merge-group-by-use=false -filetype=obj < %s | llvm-dwarfdump -debug-dump=info - | FileCheck %s
 
 ; Check debug info output for merged global.
 ; DW_AT_location
 ; 0x03 DW_OP_addr
 ; 0x.. .long __MergedGlobals
 ; 0x10 DW_OP_constu
 ; 0x.. offset
 ; 0x22 DW_OP_plus
 
 ; CHECK: DW_TAG_variable
 ; CHECK-NOT: DW_TAG
 ; CHECK:    DW_AT_name {{.*}} "x1"
 ; CHECK-NOT: {{DW_TAG|NULL}}
-; CHECK:    DW_AT_location [DW_FORM_exprloc]        (<0x8> 03 [[ADDR:.. .. .. ..]] 10 00 22  )
+; CHECK:    DW_AT_location [DW_FORM_exprloc]        (<0x5> 03 [[ADDR:.. .. .. ..]]   )
 ; CHECK: DW_TAG_variable
 ; CHECK-NOT: DW_TAG
 ; CHECK:    DW_AT_name {{.*}} "x2"
 ; CHECK-NOT: {{DW_TAG|NULL}}
 ; CHECK:    DW_AT_location [DW_FORM_exprloc]        (<0x8> 03 [[ADDR]] 10 04 22  )
 
 target datalayout = "e-p:32:32:32-i1:8:32-i8:8:32-i16:16:32-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:32:64-v128:32:128-a0:0:32-n32"
 target triple = "thumbv7-apple-macosx10.7.0"
 
-@x1 = internal unnamed_addr global i32 1, align 4
-@x2 = internal unnamed_addr global i32 2, align 4
+@x1 = internal unnamed_addr global i32 1, align 4, !dbg 0 !25
+@x2 = internal unnamed_addr global i32 2, align 4, !dbg 0 !26
 @x3 = internal unnamed_addr global i32 3, align 4
 @x4 = internal unnamed_addr global i32 4, align 4
 @x5 = global i32 0, align 4
 
-define i32 @get1(i32 %a) nounwind optsize ssp !dbg !1 {
+define i32 @get1(i32 %a) nounwind optsize ssp !dbg 0 !1 {
   tail call void @llvm.dbg.value(metadata i32 %a, i64 0, metadata !10, metadata !DIExpression()), !dbg !30
   %1 = load i32, i32* @x1, align 4, !dbg !31
   tail call void @llvm.dbg.value(metadata i32 %1, i64 0, metadata !11, metadata !DIExpression()), !dbg !31
   store i32 %a, i32* @x1, align 4, !dbg !31
   ret i32 %1, !dbg !31
 }
 
-define i32 @get2(i32 %a) nounwind optsize ssp !dbg !6 {
+define i32 @get2(i32 %a) nounwind optsize ssp !dbg 0 !6 {
   tail call void @llvm.dbg.value(metadata i32 %a, i64 0, metadata !13, metadata !DIExpression()), !dbg !32
   %1 = load i32, i32* @x2, align 4, !dbg !33
   tail call void @llvm.dbg.value(metadata i32 %1, i64 0, metadata !14, metadata !DIExpression()), !dbg !33
   store i32 %a, i32* @x2, align 4, !dbg !33
   ret i32 %1, !dbg !33
 }
 
-define i32 @get3(i32 %a) nounwind optsize ssp !dbg !7 {
+define i32 @get3(i32 %a) nounwind optsize ssp !dbg 0 !7 {
   tail call void @llvm.dbg.value(metadata i32 %a, i64 0, metadata !16, metadata !DIExpression()), !dbg !34
   %1 = load i32, i32* @x3, align 4, !dbg !35
   tail call void @llvm.dbg.value(metadata i32 %1, i64 0, metadata !17, metadata !DIExpression()), !dbg !35
   store i32 %a, i32* @x3, align 4, !dbg !35
   ret i32 %1, !dbg !35
 }
 
-define i32 @get4(i32 %a) nounwind optsize ssp !dbg !8 {
+define i32 @get4(i32 %a) nounwind optsize ssp !dbg 0 !8 {
   tail call void @llvm.dbg.value(metadata i32 %a, i64 0, metadata !19, metadata !DIExpression()), !dbg !36
   %1 = load i32, i32* @x4, align 4, !dbg !37
   tail call void @llvm.dbg.value(metadata i32 %1, i64 0, metadata !20, metadata !DIExpression()), !dbg !37
   store i32 %a, i32* @x4, align 4, !dbg !37
   ret i32 %1, !dbg !37
 }
 
-define i32 @get5(i32 %a) nounwind optsize ssp !dbg !9 {
+define i32 @get5(i32 %a) nounwind optsize ssp !dbg 0 !9 {
   tail call void @llvm.dbg.value(metadata i32 %a, i64 0, metadata !27, metadata !DIExpression()), !dbg !38
   %1 = load i32, i32* @x5, align 4, !dbg !39
   tail call void @llvm.dbg.value(metadata i32 %1, i64 0, metadata !28, metadata !DIExpression()), !dbg !39
   store i32 %a, i32* @x5, align 4, !dbg !39
   ret i32 %1, !dbg !39
 }
 
 declare void @llvm.dbg.value(metadata, i64, metadata, metadata) nounwind readnone

 !14 = !DILocalVariable(name: "b", line: 8, scope: !15, file: !2, type: !5)
 !15 = distinct !DILexicalBlock(line: 8, column: 17, file: !47, scope: !6)
 !16 = !DILocalVariable(name: "a", line: 11, arg: 1, scope: !7, file: !2, type: !5)
 !17 = !DILocalVariable(name: "b", line: 11, scope: !18, file: !2, type: !5)
 !18 = distinct !DILexicalBlock(line: 11, column: 19, file: !47, scope: !7)
 !19 = !DILocalVariable(name: "a", line: 14, arg: 1, scope: !8, file: !2, type: !5)
 !20 = !DILocalVariable(name: "b", line: 14, scope: !21, file: !2, type: !5)
 !21 = distinct !DILexicalBlock(line: 14, column: 19, file: !47, scope: !8)
-!25 = !DIGlobalVariable(name: "x1", line: 4, isLocal: true, isDefinition: true, scope: !0, file: !2, type: !5, variable: i32* @x1)
-!26 = !DIGlobalVariable(name: "x2", line: 7, isLocal: true, isDefinition: true, scope: !0, file: !2, type: !5, variable: i32* @x2)
+!25 = !DIGlobalVariable(name: "x1", line: 4, isLocal: true, isDefinition: true, scope: !0, file: !2, type: !5)
+!26 = !DIGlobalVariable(name: "x2", line: 7, isLocal: true, isDefinition: true, scope: !0, file: !2, type: !5)
 !27 = !DILocalVariable(name: "a", line: 17, arg: 1, scope: !9, file: !2, type: !5)
 !28 = !DILocalVariable(name: "b", line: 17, scope: !29, file: !2, type: !5)
 !29 = distinct !DILexicalBlock(line: 17, column: 19, file: !47, scope: !9)
 !30 = !DILocation(line: 5, column: 16, scope: !1)
 !31 = !DILocation(line: 5, column: 32, scope: !12)
 !32 = !DILocation(line: 8, column: 14, scope: !6)
 !33 = !DILocation(line: 8, column: 29, scope: !15)
 !34 = !DILocation(line: 11, column: 16, scope: !7)