[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

503

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

1827

... or is the (constant) variable.

1830

Oh yes :-)

1867

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

2607

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

126

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

[pcc]

• Address review comments

lib/Bitcode/Reader/BitcodeReader.cpp

2607

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

444

Maybe this should default to nullptr?

include/llvm/IR/GlobalVariable.h

32

Why Module?

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

179–180

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.

491

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

?

lib/CodeGen/GlobalMerge.cpp

460

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

lib/IR/DebugInfoMetadata.cpp

566

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

179–180

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

491

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

lib/IR/DebugInfoMetadata.cpp

566

Okay, adjusted to say what we currently support.

[aprantl]

lib/IR/DebugInfoMetadata.cpp

566

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

2604

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

lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

129

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

2604

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

129

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 Expr        The location of the global relative to the attached
+    ///                    GlobalVariable.
     /// \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,
+                                           DIExpression *Expr,
                                            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,
+        unsigned LineNo, DIType *Ty, bool isLocalToUnit, DIExpression *Expr,
         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.
     ///
public:
     /// Create a descriptor to describe one part
     /// of aggregate variable that is fragmented across multiple Values.
     ///
     /// \param OffsetInBits Offset of the piece in bits.
     /// \param SizeInBits   Size of the piece in bits.
     DIExpression *createBitPieceExpression(unsigned OffsetInBits,
                                            unsigned SizeInBits);
 
+    /// Create an expression for a variable that does not have an address, but
+    /// does have a constant value.
+    DIExpression *createConstantValueExpression(uint64_t Val) {
+      return DIExpression::get(
+          VMContext, {dwarf::DW_OP_constu, Val, dwarf::DW_OP_stack_value});
+    }
+
     /// 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.

include/llvm/IR/DebugInfoMetadata.h

public:
   Metadata *getRawType() const { return getOperand(3); }
 
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == DILocalVariableKind ||
            MD->getMetadataID() == DIGlobalVariableKind;
   }
 };
 
+/// \brief DWARF expression.
+///
+/// This is (almost) a DWARF expression that modifies the location of a
+/// variable or (or the location of a single piece of a variable).
+///
+/// FIXME: Instead of DW_OP_plus taking an argument, this should use DW_OP_const
+/// and have DW_OP_plus consume the topmost elements on the stack.
+///
+/// TODO: Co-allocate the expression elements.
+/// TODO: Separate from MDNode, or otherwise drop Distinct and Temporary
+/// storage types.
+class DIExpression : public MDNode {
+  friend class LLVMContextImpl;
+  friend class MDNode;
+
+  std::vector<uint64_t> Elements;
+
+  DIExpression(LLVMContext &C, StorageType Storage, ArrayRef<uint64_t> Elements)
+      : MDNode(C, DIExpressionKind, Storage, None),
+        Elements(Elements.begin(), Elements.end()) {}
+  ~DIExpression() = default;
+
+  static DIExpression *getImpl(LLVMContext &Context,
+                               ArrayRef<uint64_t> Elements, StorageType Storage,
+                               bool ShouldCreate = true);
+
+  TempDIExpression cloneImpl() const {
+    return getTemporary(getContext(), getElements());
+  }
+
+public:
+  DEFINE_MDNODE_GET(DIExpression, (ArrayRef<uint64_t> Elements), (Elements))
+
+  TempDIExpression clone() const { return cloneImpl(); }
+
+  ArrayRef<uint64_t> getElements() const { return Elements; }
+
+  unsigned getNumElements() const { return Elements.size(); }
+  uint64_t getElement(unsigned I) const {
+    assert(I < Elements.size() && "Index out of range");
+    return Elements[I];
+  }
+
+  /// \brief Return whether this is a piece of an aggregate variable.
+  bool isBitPiece() const;
+
+  /// \brief Return the offset of this piece in bits.
+  uint64_t getBitPieceOffset() const;
+
+  /// \brief Return the size of this piece in bits.
+  uint64_t getBitPieceSize() const;
+
+  typedef ArrayRef<uint64_t>::iterator element_iterator;
+  element_iterator elements_begin() const { return getElements().begin(); }
+  element_iterator elements_end() const { return getElements().end(); }
+
+  /// \brief A lightweight wrapper around an expression operand.
+  ///
+  /// TODO: Store arguments directly and change \a DIExpression to store a
+  /// range of these.
+  class ExprOperand {
+    const uint64_t *Op;
+
+  public:
+    explicit ExprOperand(const uint64_t *Op) : Op(Op) {}
+
+    const uint64_t *get() const { return Op; }
+
+    /// \brief Get the operand code.
+    uint64_t getOp() const { return *Op; }
+
+    /// \brief Get an argument to the operand.
+    ///
+    /// Never returns the operand itself.
+    uint64_t getArg(unsigned I) const { return Op[I + 1]; }
+
+    unsigned getNumArgs() const { return getSize() - 1; }
+
+    /// \brief Return the size of the operand.
+    ///
+    /// Return the number of elements in the operand (1 + args).
+    unsigned getSize() const;
+  };
+
+  /// \brief An iterator for expression operands.
+  class expr_op_iterator
+      : public std::iterator<std::input_iterator_tag, ExprOperand> {
+    ExprOperand Op;
+
+  public:
+    explicit expr_op_iterator(element_iterator I) : Op(I) {}
+
+    element_iterator getBase() const { return Op.get(); }
+    const ExprOperand &operator*() const { return Op; }
+    const ExprOperand *operator->() const { return &Op; }
+
+    expr_op_iterator &operator++() {
+      increment();
+      return *this;
+    }
+    expr_op_iterator operator++(int) {
+      expr_op_iterator T(*this);
+      increment();
+      return T;
+    }
+
+    /// \brief Get the next iterator.
+    ///
+    /// \a std::next() doesn't work because this is technically an
+    /// input_iterator, but it's a perfectly valid operation.  This is an
+    /// accessor to provide the same functionality.
+    expr_op_iterator getNext() const { return ++expr_op_iterator(*this); }
+
+    bool operator==(const expr_op_iterator &X) const {
+      return getBase() == X.getBase();
+    }
+    bool operator!=(const expr_op_iterator &X) const {
+      return getBase() != X.getBase();
+    }
+
+  private:
+    void increment() { Op = ExprOperand(getBase() + Op.getSize()); }
+  };
+
+  /// \brief Visit the elements via ExprOperand wrappers.
+  ///
+  /// These range iterators visit elements through \a ExprOperand wrappers.
+  /// This is not guaranteed to be a valid range unless \a isValid() gives \c
+  /// true.
+  ///
+  /// \pre \a isValid() gives \c true.
+  /// @{
+  expr_op_iterator expr_op_begin() const {
+    return expr_op_iterator(elements_begin());
+  }
+  expr_op_iterator expr_op_end() const {
+    return expr_op_iterator(elements_end());
+  }
+  /// @}
+
+  bool isValid() const;
+
+  static bool classof(const Metadata *MD) {
+    return MD->getMetadataID() == DIExpressionKind;
+  }
+};
+
 /// \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,
+          bool IsLocalToUnit, bool IsDefinition, DIExpression *Expr,
           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,
+                   IsLocalToUnit, IsDefinition, Expr,
                    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 *Expr,
           Metadata *StaticDataMemberDeclaration, StorageType Storage,
           bool ShouldCreate = true);
 
   TempDIGlobalVariable cloneImpl() const {
     return getTemporary(getContext(), getScope(), getName(), getLinkageName(),
                         getFile(), getLine(), getType(), isLocalToUnit(),
-                        isDefinition(), getVariable(),
+                        isDefinition(), getExpr(),
                         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, DIExpression *Expr,
                      DIDerivedType *StaticDataMemberDeclaration),
                     (Scope, Name, LinkageName, File, Line, Type, IsLocalToUnit,
-                     IsDefinition, Variable, StaticDataMemberDeclaration))
+                     IsDefinition, Expr, 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 *Expr,
                      Metadata *StaticDataMemberDeclaration),
                     (Scope, Name, LinkageName, File, Line, Type, IsLocalToUnit,
-                     IsDefinition, Variable, StaticDataMemberDeclaration))
+                     IsDefinition, Expr, 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());
-    return nullptr;
+  DIExpression *getExpr() const {
+    return cast_or_null<DIExpression>(getRawExpr());
+  }
+  void replaceExpr(DIExpression *E) {
+    replaceOperandWith(6, E);
   }
   DIDerivedType *getStaticDataMemberDeclaration() const {
     return cast_or_null<DIDerivedType>(getRawStaticDataMemberDeclaration());
   }
 
   MDString *getRawLinkageName() const { return getOperandAs<MDString>(5); }
-  Metadata *getRawVariable() const { return getOperand(6); }
+  Metadata *getRawExpr() const { return getOperand(6); }
   Metadata *getRawStaticDataMemberDeclaration() const { return getOperand(7); }
 
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == DIGlobalVariableKind;
   }
 };
 
 /// \brief Local variable.
bool isValidLocationForIntrinsic(const DILocation *DL) const {
     return DL && getScope()->getSubprogram() == DL->getScope()->getSubprogram();
   }
 
   static bool classof(const Metadata *MD) {
     return MD->getMetadataID() == DILocalVariableKind;
   }
 };
 
-/// \brief DWARF expression.
-///
-/// This is (almost) a DWARF expression that modifies the location of a
-/// variable or (or the location of a single piece of a variable).
-///
-/// FIXME: Instead of DW_OP_plus taking an argument, this should use DW_OP_const
-/// and have DW_OP_plus consume the topmost elements on the stack.
-///
-/// TODO: Co-allocate the expression elements.
-/// TODO: Separate from MDNode, or otherwise drop Distinct and Temporary
-/// storage types.
-class DIExpression : public MDNode {
-  friend class LLVMContextImpl;
-  friend class MDNode;
-
-  std::vector<uint64_t> Elements;
-
-  DIExpression(LLVMContext &C, StorageType Storage, ArrayRef<uint64_t> Elements)
-      : MDNode(C, DIExpressionKind, Storage, None),
-        Elements(Elements.begin(), Elements.end()) {}
-  ~DIExpression() = default;
-
-  static DIExpression *getImpl(LLVMContext &Context,
-                               ArrayRef<uint64_t> Elements, StorageType Storage,
-                               bool ShouldCreate = true);
-
-  TempDIExpression cloneImpl() const {
-    return getTemporary(getContext(), getElements());
-  }
-
-public:
-  DEFINE_MDNODE_GET(DIExpression, (ArrayRef<uint64_t> Elements), (Elements))
-
-  TempDIExpression clone() const { return cloneImpl(); }
-
-  ArrayRef<uint64_t> getElements() const { return Elements; }
-
-  unsigned getNumElements() const { return Elements.size(); }
-  uint64_t getElement(unsigned I) const {
-    assert(I < Elements.size() && "Index out of range");
-    return Elements[I];
-  }
-
-  /// \brief Return whether this is a piece of an aggregate variable.
-  bool isBitPiece() const;
-
-  /// \brief Return the offset of this piece in bits.
-  uint64_t getBitPieceOffset() const;
-
-  /// \brief Return the size of this piece in bits.
-  uint64_t getBitPieceSize() const;
-
-  typedef ArrayRef<uint64_t>::iterator element_iterator;
-  element_iterator elements_begin() const { return getElements().begin(); }
-  element_iterator elements_end() const { return getElements().end(); }
-
-  /// \brief A lightweight wrapper around an expression operand.
-  ///
-  /// TODO: Store arguments directly and change \a DIExpression to store a
-  /// range of these.
-  class ExprOperand {
-    const uint64_t *Op;
-
-  public:
-    explicit ExprOperand(const uint64_t *Op) : Op(Op) {}
-
-    const uint64_t *get() const { return Op; }
-
-    /// \brief Get the operand code.
-    uint64_t getOp() const { return *Op; }
-
-    /// \brief Get an argument to the operand.
-    ///
-    /// Never returns the operand itself.
-    uint64_t getArg(unsigned I) const { return Op[I + 1]; }
-
-    unsigned getNumArgs() const { return getSize() - 1; }
-
-    /// \brief Return the size of the operand.
-    ///
-    /// Return the number of elements in the operand (1 + args).
-    unsigned getSize() const;
-  };
-
-  /// \brief An iterator for expression operands.
-  class expr_op_iterator
-      : public std::iterator<std::input_iterator_tag, ExprOperand> {
-    ExprOperand Op;
-
-  public:
-    explicit expr_op_iterator(element_iterator I) : Op(I) {}
-
-    element_iterator getBase() const { return Op.get(); }
-    const ExprOperand &operator*() const { return Op; }
-    const ExprOperand *operator->() const { return &Op; }
-
-    expr_op_iterator &operator++() {
-      increment();
-      return *this;
-    }
-    expr_op_iterator operator++(int) {
-      expr_op_iterator T(*this);
-      increment();
-      return T;
-    }
-
-    /// \brief Get the next iterator.
-    ///
-    /// \a std::next() doesn't work because this is technically an
-    /// input_iterator, but it's a perfectly valid operation.  This is an
-    /// accessor to provide the same functionality.
-    expr_op_iterator getNext() const { return ++expr_op_iterator(*this); }
-
-    bool operator==(const expr_op_iterator &X) const {
-      return getBase() == X.getBase();
-    }
-    bool operator!=(const expr_op_iterator &X) const {
-      return getBase() != X.getBase();
-    }
-
-  private:
-    void increment() { Op = ExprOperand(getBase() + Op.getSize()); }
-  };
-
-  /// \brief Visit the elements via ExprOperand wrappers.
-  ///
-  /// These range iterators visit elements through \a ExprOperand wrappers.
-  /// This is not guaranteed to be a valid range unless \a isValid() gives \c
-  /// true.
-  ///
-  /// \pre \a isValid() gives \c true.
-  /// @{
-  expr_op_iterator expr_op_begin() const {
-    return expr_op_iterator(elements_begin());
-  }
-  expr_op_iterator expr_op_end() const {
-    return expr_op_iterator(elements_end());
-  }
-  /// @}
-
-  bool isValid() const;
-
-  static bool classof(const Metadata *MD) {
-    return MD->getMetadataID() == DIExpressionKind;
-  }
-};
-
 class DIObjCProperty : public DINode {
   friend class LLVMContextImpl;
   friend class MDNode;
 
   unsigned Line;
   unsigned Attributes;
 
   DIObjCProperty(LLVMContext &C, StorageType Storage, unsigned Line,

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<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_TEMPLATE_VALUE: {
           NextMetadataNo++);
       break;
     }
     case bitc::METADATA_GLOBAL_VAR: {
       if (Record.size() != 11)
         return error("Invalid record");
 
       IsDistinct = Record[0];
-      MetadataList.assignValue(
-          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++);
+
+      // Upgrade old metadata, which stored a global variable reference or a
+      // ConstantInt here.
+      Metadata *Expr = getMDOrNull(Record[9]);
+      GlobalVariable *Attach = nullptr;
+      if (auto *CMD = dyn_cast_or_null<ConstantAsMetadata>(Expr)) {
+        if (auto *GV = dyn_cast<GlobalVariable>(CMD->getValue())) {
+          Attach = GV;
+          Expr = nullptr;
+        } else if (auto *CI = dyn_cast<ConstantInt>(CMD->getValue())) {
+          Expr = DIExpression::get(Context,
+                                   {dwarf::DW_OP_constu, CI->getZExtValue(),
+                                    dwarf::DW_OP_stack_value});
+        } else {
+          Expr = nullptr;
+        }
+      }
+
+      DIGlobalVariable *DGV = 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], Expr,
+           getMDOrNull(Record[10])));
+      MetadataList.assignValue(DGV, NextMetadataNo++);
+
+      if (Attach)
+        Attach->addDebugInfo(DGV);
+
       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

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) {
   // 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())) {
-    addToAccelTable = true;
+
+  DIExpression *Expr = GV->getExpr();
+
+  // For compatibility with DWARF 4,
+  // DW_AT_location(DW_OP_constu, X, DW_OP_stack_value) becomes
+  // DW_AT_const_value(X).
+  if (Expr && Expr->getNumElements() == 3 &&
+      Expr->getElement(0) == dwarf::DW_OP_constu &&
+      Expr->getElement(2) == dwarf::DW_OP_stack_value) {
+    addConstantValue(*VariableDIE, /*Unsigned=*/true, Expr->getElement(1));
+  } else {
     DIELoc *Loc = new (DIEValueAllocator) DIELoc;
+    if (Global) {
+      addToAccelTable = true;
       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
                                                     ? dwarf::DW_OP_const4u
                                                     : dwarf::DW_OP_const8u);
             // 2) containing the (relocated) offset of the TLS variable
             //    within the module's TLS block.
             addExpr(*Loc, dwarf::DW_FORM_udata,
                     Asm->getObjFileLowering().getDebugThreadLocalSymbol(Sym));
           } else {
             addUInt(*Loc, dwarf::DW_FORM_data1, dwarf::DW_OP_GNU_const_index);
             addUInt(*Loc, dwarf::DW_FORM_udata,
                     DD->getAddressPool().getIndex(Sym, /* TLS */ true));
           }
           // 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 (Expr) {
+        DIEDwarfExpression DwarfExpr(*Asm, *this, *Loc);
+        DwarfExpr.AddExpression(Expr->expr_op_begin(), Expr->expr_op_end());
+      }
+    }
+
     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())) {
-    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);
   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

 // the target AsmPrinter.
 void DwarfDebug::beginModule() {
   NamedRegionTimer T(DbgTimerName, DWARFGroupName, TimePassesIsEnabled);
   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 *, const GlobalVariable *> GVMap;
+  for (const GlobalVariable &Global : M->globals()) {
+    SmallVector<DIGlobalVariable *, 1> GVs;
+    Global.getDebugInfo(GVs);
+    for (auto &GV : GVs)
+      GVMap[GV] = &Global;
+  }
+
   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;
+      auto I = GVMap.find(GV);
+      if (I != GVMap.end())
+        Global = I->second;
+      CU.getOrCreateGlobalVariableDIE(GV, Global);
+    }
     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();

lib/IR/DebugInfoMetadata.cpp

DIExpression *DIExpression::getImpl(LLVMContext &Context,
   DEFINE_GETIMPL_LOOKUP(DIExpression, (Elements));
   DEFINE_GETIMPL_STORE_NO_OPS(DIExpression, (Elements));
 }
 
 unsigned DIExpression::ExprOperand::getSize() const {
   switch (getOp()) {
   case dwarf::DW_OP_bit_piece:
     return 3;
+  case dwarf::DW_OP_constu:
   case dwarf::DW_OP_plus:
   case dwarf::DW_OP_minus:
     return 2;
   default:
     return 1;
   }
 }
 
 bool DIExpression::isValid() const {
   for (auto I = expr_op_begin(), E = expr_op_end(); I != E; ++I) {
     // Check that there's space for the operand.
     if (I->get() + I->getSize() > E->get())
       return false;
 
     // Check that the operand is valid.
     switch (I->getOp()) {
     default:
       return false;
     case dwarf::DW_OP_bit_piece:
-      // Piece expressions must be at the end.
+    case dwarf::DW_OP_stack_value:
+      // Piece and stack value expressions must be at the end.
       return I->get() + I->getSize() == E->get();
+    case dwarf::DW_OP_constu:
     case dwarf::DW_OP_plus:
     case dwarf::DW_OP_minus:
     case dwarf::DW_OP_deref:
       break;
     }
   }
   return true;
 }

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  )
+; CHECK:    DW_AT_location [DW_FORM_exprloc]        (<0x7> 03 [[ADDR]] 23 04  )
 
 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 !25
+@x2 = internal unnamed_addr global i32 2, align 4, !dbg !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 {
   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

 !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)