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

[Adrian Prantl via Phabricator via llvm-commits]

aprantl 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
----------------
dblaikie wrote:
> 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.
`If the compile unit the global variable gets emitted into doesn't matter (such as a linkonce_odr symbol), ...`


================
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.
----------------
dblaikie wrote:
> 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.
Good catch, I should implement this.


================
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.
+
----------------
dblaikie wrote:
> 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?
We added the unit field to DIGlobalVariables definitions (hanging off a global's !dbg attachment) so we can efficiently determine where to emit it. It's not strictly necessary, but it mirrors the DISubprogram design.

A pure declaration doesn't need one (and having one would prevent uniquing the DIImportedEntity) because it will be emitted as a locationless declaration in the CU it is being imported into (which should always be a correct one).


https://reviews.llvm.org/D28005