Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mlir][debug] Support DICommonBlock. #111706

Merged
merged 4 commits into from
Oct 10, 2024
Merged

[mlir][debug] Support DICommonBlock. #111706

merged 4 commits into from
Oct 10, 2024

Conversation

abidh
Copy link
Contributor

@abidh abidh commented Oct 9, 2024

A COMMON block is a named area of memory that holds a collection of variables. Fortran subprograms may map the COMMON block memory area to a list of variables. A common block is represented in LLVM debug by DICommonBlock.

This PR adds support for this in MLIR. The changes are mostly mechanical apart from small change to access the DICompileUnit when the scope of the variable is DICommonBlock.

A COMMON block is a named area of memory that holds a collection of
variables. Fortran subprograms may map the COMMON block memory area to
a list of variables. A common block is represented in LLVM debug by
DICommonBlock.

This PR adds support for this in MLIR. The changes are mostly
mechanical apart from small change to access the DICompileUnit when the
scope of the variable is DICommonBlock.
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 9, 2024

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-llvm

Author: Abid Qadeer (abidh)

Changes

A COMMON block is a named area of memory that holds a collection of variables. Fortran subprograms may map the COMMON block memory area to a list of variables. A common block is represented in LLVM debug by DICommonBlock.

This PR adds support for this in MLIR. The changes are mostly mechanical apart from small change to access the DICompileUnit when the scope of the variable is DICommonBlock.


Full diff: https://github.com/llvm/llvm-project/pull/111706.diff

10 Files Affected:

  • (modified) mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td (+16)
  • (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp (+11-9)
  • (modified) mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp (+6-5)
  • (modified) mlir/lib/Target/LLVMIR/DebugImporter.cpp (+9)
  • (modified) mlir/lib/Target/LLVMIR/DebugImporter.h (+1)
  • (modified) mlir/lib/Target/LLVMIR/DebugTranslation.cpp (+14-6)
  • (modified) mlir/lib/Target/LLVMIR/DebugTranslation.h (+1)
  • (modified) mlir/lib/Target/LLVMIR/ModuleTranslation.cpp (+19-10)
  • (modified) mlir/test/Target/LLVMIR/Import/debug-info.ll (+24)
  • (modified) mlir/test/Target/LLVMIR/llvmir-debug.mlir (+30)
diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index 80c22a357287ba..02ecf435d58c31 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
@@ -701,6 +701,22 @@ def LLVM_DISubrangeAttr : LLVM_Attr<"DISubrange", "di_subrange", /*traits=*/[],
   let assemblyFormat = "`<` struct(params) `>`";
 }
 
+//===----------------------------------------------------------------------===//
+// DICommonBlockAttr
+//===----------------------------------------------------------------------===//
+
+def LLVM_DICommonBlockAttr : LLVM_Attr<"DICommonBlock", "di_common_block",
+                                       /*traits=*/[], "DIScopeAttr"> {
+  let parameters = (ins
+    "DIScopeAttr":$scope,
+    OptionalParameter<"DIGlobalVariableAttr">:$decl,
+    "StringAttr":$name,
+    OptionalParameter<"DIFileAttr">:$file,
+    OptionalParameter<"unsigned">:$line
+  );
+  let assemblyFormat = "`<` struct(params) `>`";
+}
+
 //===----------------------------------------------------------------------===//
 // DISubroutineTypeAttr
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
index 99871dac81d326..9640bbdf28df45 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp
@@ -56,13 +56,14 @@ void LLVMDialect::registerAttributes() {
 //===----------------------------------------------------------------------===//
 
 bool DINodeAttr::classof(Attribute attr) {
-  return llvm::isa<DIBasicTypeAttr, DICompileUnitAttr, DICompositeTypeAttr,
-                   DIDerivedTypeAttr, DIFileAttr, DIGlobalVariableAttr,
-                   DIImportedEntityAttr, DILabelAttr, DILexicalBlockAttr,
-                   DILexicalBlockFileAttr, DILocalVariableAttr, DIModuleAttr,
-                   DINamespaceAttr, DINullTypeAttr, DIAnnotationAttr,
-                   DIStringTypeAttr, DISubprogramAttr, DISubrangeAttr,
-                   DISubroutineTypeAttr>(attr);
+  return llvm::isa<DIBasicTypeAttr, DICommonBlockAttr, DICompileUnitAttr,
+                   DICompositeTypeAttr, DIDerivedTypeAttr, DIFileAttr,
+                   DIGlobalVariableAttr, DIImportedEntityAttr, DILabelAttr,
+                   DILexicalBlockAttr, DILexicalBlockFileAttr,
+                   DILocalVariableAttr, DIModuleAttr, DINamespaceAttr,
+                   DINullTypeAttr, DIAnnotationAttr, DIStringTypeAttr,
+                   DISubprogramAttr, DISubrangeAttr, DISubroutineTypeAttr>(
+      attr);
 }
 
 //===----------------------------------------------------------------------===//
@@ -70,8 +71,9 @@ bool DINodeAttr::classof(Attribute attr) {
 //===----------------------------------------------------------------------===//
 
 bool DIScopeAttr::classof(Attribute attr) {
-  return llvm::isa<DICompileUnitAttr, DICompositeTypeAttr, DIFileAttr,
-                   DILocalScopeAttr, DIModuleAttr, DINamespaceAttr>(attr);
+  return llvm::isa<DICommonBlockAttr, DICompileUnitAttr, DICompositeTypeAttr,
+                   DIFileAttr, DILocalScopeAttr, DIModuleAttr, DINamespaceAttr>(
+      attr);
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
index 9e361848f8c0bf..1b81a6efea1894 100644
--- a/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
+++ b/mlir/lib/Dialect/LLVMIR/IR/LLVMDialect.cpp
@@ -3339,11 +3339,12 @@ struct LLVMOpAsmDialectInterface : public OpAsmDialectInterface {
   AliasResult getAlias(Attribute attr, raw_ostream &os) const override {
     return TypeSwitch<Attribute, AliasResult>(attr)
         .Case<AccessGroupAttr, AliasScopeAttr, AliasScopeDomainAttr,
-              DIBasicTypeAttr, DICompileUnitAttr, DICompositeTypeAttr,
-              DIDerivedTypeAttr, DIFileAttr, DIGlobalVariableAttr,
-              DIGlobalVariableExpressionAttr, DIImportedEntityAttr, DILabelAttr,
-              DILexicalBlockAttr, DILexicalBlockFileAttr, DILocalVariableAttr,
-              DIModuleAttr, DINamespaceAttr, DINullTypeAttr, DIStringTypeAttr,
+              DIBasicTypeAttr, DICommonBlockAttr, DICompileUnitAttr,
+              DICompositeTypeAttr, DIDerivedTypeAttr, DIFileAttr,
+              DIGlobalVariableAttr, DIGlobalVariableExpressionAttr,
+              DIImportedEntityAttr, DILabelAttr, DILexicalBlockAttr,
+              DILexicalBlockFileAttr, DILocalVariableAttr, DIModuleAttr,
+              DINamespaceAttr, DINullTypeAttr, DIStringTypeAttr,
               DISubprogramAttr, DISubroutineTypeAttr, LoopAnnotationAttr,
               LoopVectorizeAttr, LoopInterleaveAttr, LoopUnrollAttr,
               LoopUnrollAndJamAttr, LoopLICMAttr, LoopDistributeAttr,
diff --git a/mlir/lib/Target/LLVMIR/DebugImporter.cpp b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
index cd992be62b4719..412125b6ea65f6 100644
--- a/mlir/lib/Target/LLVMIR/DebugImporter.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
@@ -302,6 +302,13 @@ DISubrangeAttr DebugImporter::translateImpl(llvm::DISubrange *node) {
                              getAttrOrNull(node->getStride()));
 }
 
+DICommonBlockAttr DebugImporter::translateImpl(llvm::DICommonBlock *node) {
+  return DICommonBlockAttr::get(context, translate(node->getScope()),
+                                translate(node->getDecl()),
+                                getStringAttrOrNull(node->getRawName()),
+                                translate(node->getFile()), node->getLineNo());
+}
+
 DISubroutineTypeAttr
 DebugImporter::translateImpl(llvm::DISubroutineType *node) {
   SmallVector<DITypeAttr> types;
@@ -339,6 +346,8 @@ DINodeAttr DebugImporter::translate(llvm::DINode *node) {
   auto translateNode = [this](llvm::DINode *node) -> DINodeAttr {
     if (auto *casted = dyn_cast<llvm::DIBasicType>(node))
       return translateImpl(casted);
+    if (auto *casted = dyn_cast<llvm::DICommonBlock>(node))
+      return translateImpl(casted);
     if (auto *casted = dyn_cast<llvm::DICompileUnit>(node))
       return translateImpl(casted);
     if (auto *casted = dyn_cast<llvm::DICompositeType>(node))
diff --git a/mlir/lib/Target/LLVMIR/DebugImporter.h b/mlir/lib/Target/LLVMIR/DebugImporter.h
index cb796676759c39..a452e01a9f6041 100644
--- a/mlir/lib/Target/LLVMIR/DebugImporter.h
+++ b/mlir/lib/Target/LLVMIR/DebugImporter.h
@@ -79,6 +79,7 @@ class DebugImporter {
   DIScopeAttr translateImpl(llvm::DIScope *node);
   DISubprogramAttr translateImpl(llvm::DISubprogram *node);
   DISubrangeAttr translateImpl(llvm::DISubrange *node);
+  DICommonBlockAttr translateImpl(llvm::DICommonBlock *node);
   DISubroutineTypeAttr translateImpl(llvm::DISubroutineType *node);
   DITypeAttr translateImpl(llvm::DIType *node);
 
diff --git a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
index 92ff079a10c8aa..2491db299af312 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
@@ -397,6 +397,13 @@ llvm::DISubrange *DebugTranslation::translateImpl(DISubrangeAttr attr) {
                                getMetadataOrNull(attr.getStride()));
 }
 
+llvm::DICommonBlock *DebugTranslation::translateImpl(DICommonBlockAttr attr) {
+  return llvm::DICommonBlock::get(llvmCtx, translate(attr.getScope()),
+                                  translate(attr.getDecl()),
+                                  getMDStringOrNull(attr.getName()),
+                                  translate(attr.getFile()), attr.getLine());
+}
+
 llvm::DISubroutineType *
 DebugTranslation::translateImpl(DISubroutineTypeAttr attr) {
   // Concatenate the result and argument types into a single array.
@@ -428,12 +435,13 @@ llvm::DINode *DebugTranslation::translate(DINodeAttr attr) {
 
   if (!node)
     node = TypeSwitch<DINodeAttr, llvm::DINode *>(attr)
-               .Case<DIBasicTypeAttr, DICompileUnitAttr, DICompositeTypeAttr,
-                     DIDerivedTypeAttr, DIFileAttr, DIGlobalVariableAttr,
-                     DIImportedEntityAttr, DILabelAttr, DILexicalBlockAttr,
-                     DILexicalBlockFileAttr, DILocalVariableAttr, DIModuleAttr,
-                     DINamespaceAttr, DINullTypeAttr, DIStringTypeAttr,
-                     DISubprogramAttr, DISubrangeAttr, DISubroutineTypeAttr>(
+               .Case<DIBasicTypeAttr, DICommonBlockAttr, DICompileUnitAttr,
+                     DICompositeTypeAttr, DIDerivedTypeAttr, DIFileAttr,
+                     DIGlobalVariableAttr, DIImportedEntityAttr, DILabelAttr,
+                     DILexicalBlockAttr, DILexicalBlockFileAttr,
+                     DILocalVariableAttr, DIModuleAttr, DINamespaceAttr,
+                     DINullTypeAttr, DIStringTypeAttr, DISubprogramAttr,
+                     DISubrangeAttr, DISubroutineTypeAttr>(
                    [&](auto attr) { return translateImpl(attr); });
 
   if (node && !node->isTemporary())
diff --git a/mlir/lib/Target/LLVMIR/DebugTranslation.h b/mlir/lib/Target/LLVMIR/DebugTranslation.h
index 422aa34e28f3c9..ff4eaa46c564e2 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.h
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.h
@@ -88,6 +88,7 @@ class DebugTranslation {
   llvm::DIScope *translateImpl(DIScopeAttr attr);
   llvm::DISubprogram *translateImpl(DISubprogramAttr attr);
   llvm::DISubrange *translateImpl(DISubrangeAttr attr);
+  llvm::DICommonBlock *translateImpl(DICommonBlockAttr attr);
   llvm::DISubroutineType *translateImpl(DISubroutineTypeAttr attr);
   llvm::DIType *translateImpl(DITypeAttr attr);
 
diff --git a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
index cc0de5bc838c99..08bd37e1b8a976 100644
--- a/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/ModuleTranslation.cpp
@@ -1064,19 +1064,28 @@ LogicalResult ModuleTranslation::convertGlobals() {
       // There is no `globals` field in DICompileUnitAttr which can be directly
       // assigned to DICompileUnit. We have to build the list by looking at the
       // dbgExpr of all the GlobalOps. The scope of the variable is used to get
-      // the DICompileUnit in which to add it. But for the languages that
-      // support modules, the scope hierarchy can be
-      // variable -> module -> compile unit
-      // If a variable scope points to the module then we use the scope of the
-      // module to get the compile unit.
-      // Global variables are also used for things like static local variables
-      // in C and local variables with the save attribute in Fortran. The scope
-      // of the variable is the parent function. We use the compile unit of the
-      // parent function in this case.
+      // the DICompileUnit in which to add it.
+      // But there are cases where the scope of the global points does not
+      // directly point to the DICompileUnit and we have to do a bit more work
+      // to get to it. Some of those cases are:
+      //
+      // 1. For the languages that support modules, the scope hierarchy can be
+      // variable -> DIModule -> DICompileUnit
+      //
+      // 2. For the Fortran common block variable, the scope hierarchy looks
+      // like as follows:
+      // variable -> DICommonBlock -> DISubprogram -> DICompileUnit
+      //
+      // 3. For entities like static local variables in C or variable with
+      // SAVE attribute in Fortran, the scope hierarchy can be
+      // variable -> DISubprogram -> DICompileUnit
       llvm::DIScope *scope = diGlobalVar->getScope();
       if (auto *mod = dyn_cast_if_present<llvm::DIModule>(scope))
         scope = mod->getScope();
-      else if (auto *sp = dyn_cast_if_present<llvm::DISubprogram>(scope))
+      else if (auto *cb = dyn_cast_if_present<llvm::DICommonBlock>(scope)) {
+        if (auto *sp = dyn_cast_if_present<llvm::DISubprogram>(cb->getScope()))
+          scope = sp->getUnit();
+      } else if (auto *sp = dyn_cast_if_present<llvm::DISubprogram>(scope))
         scope = sp->getUnit();
 
       // Get the compile unit (scope) of the the global variable.
diff --git a/mlir/test/Target/LLVMIR/Import/debug-info.ll b/mlir/test/Target/LLVMIR/Import/debug-info.ll
index 6267990b0bf803..09909d7d63b2ab 100644
--- a/mlir/test/Target/LLVMIR/Import/debug-info.ll
+++ b/mlir/test/Target/LLVMIR/Import/debug-info.ll
@@ -843,3 +843,27 @@ define void @fn_with_annotations() !dbg !12 {
 
 
 ; CHECK-DAG: #llvm.di_subprogram<{{.*}}name = "fn_with_annotations"{{.*}}annotations = #llvm.di_annotation<name = "foo", value = "bar">>
+
+; // -----
+
+@block = common global [4 x i8] zeroinitializer, !dbg !0
+
+define void @test() !dbg !3 {
+  ret void
+}
+
+!llvm.module.flags = !{!10}
+!llvm.dbg.cu = !{!7}
+
+!0 = !DIGlobalVariableExpression(var: !1, expr: !DIExpression())
+!1 = distinct !DIGlobalVariable(name: "alpha", scope: !2, file: !4, type: !9)
+!2 = !DICommonBlock(scope: !3, declaration: null, name: "block", file: !4, line: 3)
+!3 = distinct !DISubprogram(name: "test", scope: !4, file: !4, spFlags: DISPFlagDefinition, unit: !7)
+!4 = !DIFile(filename: "test.f90", directory: "")
+!7 = distinct !DICompileUnit(language: DW_LANG_Fortran95, file: !4)
+!9 = !DIBasicType(name: "integer", size: 32, encoding: DW_ATE_signed)
+!10 = !{i32 2, !"Debug Info Version", i32 3}
+
+; CHECK: #[[FILE:.+]] = #llvm.di_file<"test.f90" in "">
+; CHECK: #[[SP:.+]] = #llvm.di_subprogram<{{.*}}name = "test"{{.*}}>
+; CHECK: #llvm.di_common_block<scope = #[[SP]], name = "block", file = #[[FILE]], line = 3>
diff --git a/mlir/test/Target/LLVMIR/llvmir-debug.mlir b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
index b09a60b8dcac90..826fda60c5efef 100644
--- a/mlir/test/Target/LLVMIR/llvmir-debug.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
@@ -660,3 +660,33 @@ llvm.func @string_ty(%arg0: !llvm.ptr) {
 
 // CHECK-DAG: !DIStringType(name: "character(*)", stringLength: ![[VAR:[0-9]+]], stringLengthExpression: !DIExpression(DW_OP_push_object_address, DW_OP_plus_uconst, 8), stringLocationExpression: !DIExpression(DW_OP_push_object_address, DW_OP_deref), size: 32, align: 8)
 // CHECK-DAG: ![[VAR]] = !DILocalVariable(name: "string_size"{{.*}} flags: DIFlagArtificial)
+
+// -----
+
+// Test translation of DICommonBlockAttr.
+#bt = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "int", sizeInBits = 32>
+#file = #llvm.di_file<"test.f90" in "">
+#cu = #llvm.di_compile_unit<id = distinct[0]<>, sourceLanguage = DW_LANG_C,
+ file = #file, isOptimized = false, emissionKind = Full>
+#sp = #llvm.di_subprogram<compileUnit = #cu, scope = #file, name = "test",
+ file = #file, subprogramFlags = Definition>
+#di_common_block = #llvm.di_common_block<scope = #sp, name = "block",
+ file = #file, line = 3>
+#global_var = #llvm.di_global_variable<scope = #di_common_block, name = "a",
+ file = #file, line = 2, type = #bt>
+#var_expression = #llvm.di_global_variable_expression<var = #global_var,
+ expr = <>>
+
+llvm.mlir.global common @block_(dense<0> : tensor<8xi8>)
+  {dbg_expr = #var_expression} : !llvm.array<8 x i8>
+
+llvm.func @test() {
+  llvm.return
+} loc(#loc2)
+
+#loc1 = loc("test.f90":1:0)
+#loc2 = loc(fused<#sp>[#loc1])
+
+// CHECK: !DICommonBlock(scope: ![[SCOPE:[0-9]+]], declaration: null, name: "block", file: ![[FILE:[0-9]+]], line: 3)
+// CHECK: ![[SCOPE]] = {{.*}}!DISubprogram(name: "test"{{.*}})
+// CHECK: ![[FILE]] = !DIFile(filename: "test.f90"{{.*}})

Copy link
Contributor

@Dinistro Dinistro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We usually also added LLVM dialect roundtrip tests for such additions. Apart from that, this is LGTM!

Copy link
Contributor

@gysit gysit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM modulo nits!

mlir/lib/Target/LLVMIR/ModuleTranslation.cpp Outdated Show resolved Hide resolved
mlir/lib/Target/LLVMIR/ModuleTranslation.cpp Outdated Show resolved Hide resolved
@abidh
Copy link
Contributor Author

abidh commented Oct 10, 2024

We usually also added LLVM dialect roundtrip tests for such additions. Apart from that, this is LGTM!

Thanks for the review. I was not sure where the round trip test go. But I have added one in test/Dialect/LLVMIR/debuginfo.mlir. Please let me know if that works.

@abidh abidh requested a review from Dinistro October 10, 2024 11:29
Copy link
Contributor

@Dinistro Dinistro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for addressing the comments.

@abidh abidh merged commit 36c34ec into llvm:main Oct 10, 2024
8 checks passed
ericastor pushed a commit to ericastor/llvm-project that referenced this pull request Oct 10, 2024
A COMMON block is a named area of memory that holds a collection of
variables. Fortran subprograms may map the COMMON block memory area to a
list of variables. A common block is represented in LLVM debug by
DICommonBlock.

This PR adds support for this in MLIR. The changes are mostly mechanical
apart from small change to access the DICompileUnit when the scope of
the variable is DICommonBlock.

---------

Co-authored-by: Tobias Gysi <[email protected]>
DanielCChen pushed a commit to DanielCChen/llvm-project that referenced this pull request Oct 16, 2024
A COMMON block is a named area of memory that holds a collection of
variables. Fortran subprograms may map the COMMON block memory area to a
list of variables. A common block is represented in LLVM debug by
DICommonBlock.

This PR adds support for this in MLIR. The changes are mostly mechanical
apart from small change to access the DICompileUnit when the scope of
the variable is DICommonBlock.

---------

Co-authored-by: Tobias Gysi <[email protected]>
bricknerb pushed a commit to bricknerb/llvm-project that referenced this pull request Oct 17, 2024
A COMMON block is a named area of memory that holds a collection of
variables. Fortran subprograms may map the COMMON block memory area to a
list of variables. A common block is represented in LLVM debug by
DICommonBlock.

This PR adds support for this in MLIR. The changes are mostly mechanical
apart from small change to access the DICompileUnit when the scope of
the variable is DICommonBlock.

---------

Co-authored-by: Tobias Gysi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants