[PATCH] D28005: Add a unit field to DIGlobalVariable, mirroring the design of DISubprogram.

[David Blaikie via Phabricator via llvm-commits]

dblaikie added inline comments.


================
Comment at: docs/SourceLevelDebugging.rst:452
+   the variable's DICompileUnit.  If the global symbol the variable is attached
+   to is linkonce_odr, and the compile unit the global variable gets emitted
+   into doesn't matter, the DIGlobalVariable may instead be unique (i.e., not
----------------
Probably could omit the reference to linkonce_odr and let the rest of the definition stand.

That way if anything were to duplicate other globals (eg: thinlto might import globals, etc) they could/would use this functionality as well, potentially.


================
Comment at: docs/SourceLevelDebugging.rst:456-458
+2. *Constant definitions* are reachable via the DICompileUnit's list of globals
+   which points to a DIGlobalVariableExpression with a DIGlobalVariable without
+   a unit field and a DIExpression describing a constant value.
----------------
Is this true? Do we go and change the DIGlobalVariable if we collapse an llvm::GlobalValue into a constant? So that it doesn't have a unit field anymore? I assume we don't do that, so some constants might point to DIGlobalVariables with a non-null unit field.


================
Comment at: docs/SourceLevelDebugging.rst:460-461
+
+3. *Declarations* are DIGlobalVariables without a unit field that are only
+   reachable via a DIImportedEntity and have isDefinition set to false.
+
----------------
Is this correct? If all DIGlobalVariables referenced by DIImportedEntities can have no unit field (& thus be emitted into any unit that references them) - should any DIGlobalVariables have unit fields?


https://reviews.llvm.org/D28005