[Mlir-commits] [mlir] [mlir] Add support for DIGlobalVariable and DIGlobalVariableExpression (PR #73367)

[Tobias Gysi]

================
@@ -23,15 +23,24 @@
 namespace mlir {
 namespace LLVM {
 
-/// This class represents the base attribute for all debug info attributes.
-class DINodeAttr : public Attribute {
+/// This class represents the base attribute for all metadata attributes.
+class MDNodeAttr : public Attribute {
----------------
gysit wrote:

I have seen the class diagram and i definitely don't like the name `MDNode` isn't that the shortcut for metadata node? If we want to replicate, I would at least call it DIBaseAttr or something similar. There is a lot of other metadata and `MDNode` sound very confusing to me. 

The separate translation function for the location makes sense since there is a Location class in MLIR that does not map to DILocation directly and it is a top-level class that is then attached to an operation/instruction. 

To me it seems like DIGlobalVariableExpression is only used by operations / intrinsics? That means having a function that returns a DIGlobalVariableExpression and not an MDNode base class seems beneficial for such a class. Or is there a use case where it is used in a polymorphic manner, i.e.,  another debug node that has a field which may be any kind of MDNode? If the later exists then it indeed makes sense to have a translate method that returns an MDNodeAttr. Otherwise, I am not yet convinced :).

https://github.com/llvm/llvm-project/pull/73367