[mlir][debug] Inherit DISubprogramAttr from DILocalScopeAttr.

[abidh]

As mentioned in #154926, DISubprogramAttr is inherited from DIScopeAttr while in llvm, the DISubprogram inherits from DILocalScope. This change corrects the hierarchy.

[llvmbot]

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-llvm

Author: Abid Qadeer (abidh)

Changes

As mentioned in #154926, DISubprogramAttr is inherited from DIScopeAttr while in llvm, the DISubprogram inherits from DILocalScope. This change corrects the hierarchy.

---

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

2 Files Affected:

• (modified) mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td (+1-1)
• (modified) mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp (+2-1)

diff --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index bfce904a18d4f..407078533ef5c 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
@@ -638,7 +638,7 @@ def LLVM_DILocalVariableAttr : LLVM_Attr<"DILocalVariable", "di_local_variable",
 
 def LLVM_DISubprogramAttr : LLVM_Attr<"DISubprogram", "di_subprogram",
                                       [LLVM_DIRecursiveTypeAttrInterface],
-                                      "DIScopeAttr"> {
+                                      "DILocalScopeAttr"> {
   let parameters = (ins
     // DIRecursiveTypeAttrInterface specific parameters.
     OptionalParameter<"DistinctAttr">:$recId,
diff --git a/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp
index 3b7e6eda0841d..fd8463ad1a8e2 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/LLVMIR/LLVMToLLVMIRTranslation.cpp
@@ -362,7 +362,8 @@ static void convertModuleFlagsOp(ArrayAttr flags, llvm::IRBuilderBase &builder,
 static llvm::DILocalScope *
 getLocalScopeFromLoc(llvm::IRBuilderBase &builder, Location loc,
                      LLVM::ModuleTranslation &moduleTranslation) {
-  if (auto scopeLoc = loc->findInstanceOf<FusedLocWith<LLVM::DIScopeAttr>>())
+  if (auto scopeLoc =
+          loc->findInstanceOf<FusedLocWith<LLVM::DILocalScopeAttr>>())
     if (auto *localScope = llvm::dyn_cast<llvm::DILocalScope>(
             moduleTranslation.translateDebugInfo(scopeLoc.getMetadata())))
       return localScope;

[gysit]

Thanks for the followup.

LGTM

Can you update the base class for DILexicalBlockAttr and DILexicalBlockFileAttr as well? It seems like this is an inconsistency from a previous refactor.

[gysit]

Looking at:

llvm-project/mlir/lib/Dialect/LLVMIR/IR/LLVMAttrs.cpp

Line 99 in d394353

bool DILocalScopeAttr::classof(Attribute attr) {

It seems like the base classes of DILexicalBlockAttr and DILexicalBlockFileAttr should also derive from DILocalScopeAttr rather than from DIScopeAttr. Can you update them as well?

[abidh]

Done.

[abidh]

Looking at the code a bit more, the DILexicalBlockAttr and DILexicalBlockFileAttr have scope parameters which are DIScopeAttr but probably needs to be DILocalScopeAttr. I tried a few quick changes but they soon became more invasive. I would leave them for later PR.

[gysit]

Ok 👍