[Mlir-commits] [mlir] d9391a3 - [mlir][llvm] Stop exporting empty debug MD strings

Christian Ulmann llvmlistbot at llvm.org
Mon Feb 27 00:53:28 PST 2023


Author: Christian Ulmann
Date: 2023-02-27T09:49:57+01:00
New Revision: d9391a37a9f8eb222a951606e0ca5d6a5b6b2199

URL: https://github.com/llvm/llvm-project/commit/d9391a37a9f8eb222a951606e0ca5d6a5b6b2199
DIFF: https://github.com/llvm/llvm-project/commit/d9391a37a9f8eb222a951606e0ca5d6a5b6b2199.diff

LOG: [mlir][llvm] Stop exporting empty debug MD strings

This commit ensures that no empty debug metadata strings are exported as
these are not legal names. Additionally, this commit ensures that
non-existing strings are not accidentially imported as empty strings.

Reviewed By: gysit

Differential Revision: https://reviews.llvm.org/D144263

Added: 
    

Modified: 
    mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
    mlir/lib/Target/LLVMIR/DebugImporter.cpp
    mlir/lib/Target/LLVMIR/DebugImporter.h
    mlir/lib/Target/LLVMIR/DebugTranslation.cpp
    mlir/lib/Target/LLVMIR/DebugTranslation.h
    mlir/test/Target/LLVMIR/Import/debug-info.ll
    mlir/test/Target/LLVMIR/llvmir-debug.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
index b90782bea4200..e75fd8c911828 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
+++ b/mlir/include/mlir/Dialect/LLVMIR/LLVMAttrDefs.td
@@ -303,7 +303,7 @@ def LLVM_DICompileUnitAttr : LLVM_Attr<"DICompileUnit", "di_compile_unit",
   let parameters = (ins
     LLVM_DILanguageParameter:$sourceLanguage,
     "DIFileAttr":$file,
-    "StringAttr":$producer,
+    OptionalParameter<"StringAttr">:$producer,
     "bool":$isOptimized,
     "DIEmissionKind":$emissionKind
   );
@@ -318,7 +318,7 @@ def LLVM_DICompositeTypeAttr : LLVM_Attr<"DICompositeType", "di_composite_type",
                                          /*traits=*/[], "DITypeAttr"> {
   let parameters = (ins
     LLVM_DITagParameter:$tag,
-    "StringAttr":$name,
+    OptionalParameter<"StringAttr">:$name,
     OptionalParameter<"DIFileAttr">:$file,
     OptionalParameter<"uint32_t">:$line,
     OptionalParameter<"DIScopeAttr">:$scope,
@@ -414,7 +414,7 @@ def LLVM_DILocalVariableAttr : LLVM_Attr<"DILocalVariable", "di_local_variable",
                                          /*traits=*/[], "DINodeAttr"> {
   let parameters = (ins
     "DIScopeAttr":$scope,
-    "StringAttr":$name,
+    OptionalParameter<"StringAttr">:$name,
     OptionalParameter<"DIFileAttr">:$file,
     OptionalParameter<"unsigned">:$line,
     OptionalParameter<"unsigned">:$arg,

diff  --git a/mlir/lib/Target/LLVMIR/DebugImporter.cpp b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
index 20cd5da30ae50..cff8357457cb9 100644
--- a/mlir/lib/Target/LLVMIR/DebugImporter.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugImporter.cpp
@@ -47,7 +47,7 @@ DICompileUnitAttr DebugImporter::translateImpl(llvm::DICompileUnit *node) {
       symbolizeDIEmissionKind(node->getEmissionKind());
   return DICompileUnitAttr::get(context, node->getSourceLanguage(),
                                 translate(node->getFile()),
-                                StringAttr::get(context, node->getProducer()),
+                                getStringAttrOrNull(node->getRawProducer()),
                                 node->isOptimized(), emissionKind.value());
 }
 
@@ -66,7 +66,7 @@ DICompositeTypeAttr DebugImporter::translateImpl(llvm::DICompositeType *node) {
   if (llvm::is_contained(elements, nullptr))
     elements.clear();
   return DICompositeTypeAttr::get(
-      context, node->getTag(), StringAttr::get(context, node->getName()),
+      context, node->getTag(), getStringAttrOrNull(node->getRawName()),
       translate(node->getFile()), node->getLine(), translate(node->getScope()),
       translate(node->getBaseType()), flags.value_or(DIFlags::Zero),
       node->getSizeInBits(), node->getAlignInBits(), elements);
@@ -78,8 +78,7 @@ DIDerivedTypeAttr DebugImporter::translateImpl(llvm::DIDerivedType *node) {
   if (node->getBaseType() && !baseType)
     return nullptr;
   return DIDerivedTypeAttr::get(
-      context, node->getTag(),
-      node->getRawName() ? StringAttr::get(context, node->getName()) : nullptr,
+      context, node->getTag(), getStringAttrOrNull(node->getRawName()),
       baseType, node->getSizeInBits(), node->getAlignInBits(),
       node->getOffsetInBits());
 }
@@ -103,7 +102,7 @@ DebugImporter::translateImpl(llvm::DILexicalBlockFile *node) {
 
 DILocalVariableAttr DebugImporter::translateImpl(llvm::DILocalVariable *node) {
   return DILocalVariableAttr::get(context, translate(node->getScope()),
-                                  StringAttr::get(context, node->getName()),
+                                  getStringAttrOrNull(node->getRawName()),
                                   translate(node->getFile()), node->getLine(),
                                   node->getArg(), node->getAlignInBits(),
                                   translate(node->getType()));
@@ -114,9 +113,9 @@ DIScopeAttr DebugImporter::translateImpl(llvm::DIScope *node) {
 }
 
 DINamespaceAttr DebugImporter::translateImpl(llvm::DINamespace *node) {
-  return DINamespaceAttr::get(
-      context, StringAttr::get(context, node->getName()),
-      translate(node->getScope()), node->getExportSymbols());
+  return DINamespaceAttr::get(context, getStringAttrOrNull(node->getRawName()),
+                              translate(node->getScope()),
+                              node->getExportSymbols());
 }
 
 DISubprogramAttr DebugImporter::translateImpl(llvm::DISubprogram *node) {
@@ -129,14 +128,12 @@ DISubprogramAttr DebugImporter::translateImpl(llvm::DISubprogram *node) {
   DISubroutineTypeAttr type = translate(node->getType());
   if (node->getType() && !type)
     return nullptr;
-  return DISubprogramAttr::get(
-      context, translate(node->getUnit()), scope,
-      StringAttr::get(context, node->getName()),
-      node->getRawLinkageName()
-          ? StringAttr::get(context, node->getLinkageName())
-          : nullptr,
-      translate(node->getFile()), node->getLine(), node->getScopeLine(),
-      subprogramFlags.value(), type);
+  return DISubprogramAttr::get(context, translate(node->getUnit()), scope,
+                               getStringAttrOrNull(node->getRawName()),
+                               getStringAttrOrNull(node->getRawLinkageName()),
+                               translate(node->getFile()), node->getLine(),
+                               node->getScopeLine(), subprogramFlags.value(),
+                               type);
 }
 
 DISubrangeAttr DebugImporter::translateImpl(llvm::DISubrange *node) {
@@ -248,3 +245,9 @@ Location DebugImporter::translateLoc(llvm::DILocation *loc) {
                                           context);
   return result;
 }
+
+StringAttr DebugImporter::getStringAttrOrNull(llvm::MDString *stringNode) {
+  if (!stringNode)
+    return StringAttr();
+  return StringAttr::get(context, stringNode->getString());
+}

diff  --git a/mlir/lib/Target/LLVMIR/DebugImporter.h b/mlir/lib/Target/LLVMIR/DebugImporter.h
index 2ce19bfbe0beb..e7f2332ff436d 100644
--- a/mlir/lib/Target/LLVMIR/DebugImporter.h
+++ b/mlir/lib/Target/LLVMIR/DebugImporter.h
@@ -67,6 +67,10 @@ class DebugImporter {
   DISubroutineTypeAttr translateImpl(llvm::DISubroutineType *node);
   DITypeAttr translateImpl(llvm::DIType *node);
 
+  /// Constructs a StringAttr from the MDString if it is non-null. Returns a
+  /// null attribute otherwise.
+  StringAttr getStringAttrOrNull(llvm::MDString *stringNode);
+
   /// A mapping between LLVM debug metadata and the corresponding attribute.
   DenseMap<llvm::DINode *, DINodeAttr> nodeToAttr;
 

diff  --git a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
index 478216df312a7..dae9930ea3095 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.cpp
@@ -97,17 +97,26 @@ llvm::DIType *DebugTranslation::translateImpl(DINullTypeAttr attr) {
   return nullptr;
 }
 
+llvm::MDString *DebugTranslation::getMDStringOrNull(StringAttr stringAttr) {
+  if (!stringAttr || stringAttr.getValue().empty())
+    return nullptr;
+  return llvm::MDString::get(llvmCtx, stringAttr);
+}
+
 llvm::DIBasicType *DebugTranslation::translateImpl(DIBasicTypeAttr attr) {
   return llvm::DIBasicType::get(
-      llvmCtx, attr.getTag(), attr.getName(), attr.getSizeInBits(),
+      llvmCtx, attr.getTag(), getMDStringOrNull(attr.getName()),
+      attr.getSizeInBits(),
       /*AlignInBits=*/0, attr.getEncoding(), llvm::DINode::FlagZero);
 }
 
 llvm::DICompileUnit *DebugTranslation::translateImpl(DICompileUnitAttr attr) {
   llvm::DIBuilder builder(llvmModule);
   return builder.createCompileUnit(
-      attr.getSourceLanguage(), translate(attr.getFile()), attr.getProducer(),
-      attr.getIsOptimized(), /*Flags=*/"", /*RV=*/0);
+      attr.getSourceLanguage(), translate(attr.getFile()),
+      attr.getProducer() ? attr.getProducer().getValue() : "",
+      attr.getIsOptimized(),
+      /*Flags=*/"", /*RV=*/0);
 }
 
 llvm::DICompositeType *
@@ -116,9 +125,10 @@ DebugTranslation::translateImpl(DICompositeTypeAttr attr) {
   for (auto member : attr.getElements())
     elements.push_back(translate(member));
   return llvm::DICompositeType::get(
-      llvmCtx, attr.getTag(), attr.getName(), translate(attr.getFile()),
-      attr.getLine(), translate(attr.getScope()), translate(attr.getBaseType()),
-      attr.getSizeInBits(), attr.getAlignInBits(),
+      llvmCtx, attr.getTag(), getMDStringOrNull(attr.getName()),
+      translate(attr.getFile()), attr.getLine(), translate(attr.getScope()),
+      translate(attr.getBaseType()), attr.getSizeInBits(),
+      attr.getAlignInBits(),
       /*OffsetInBits=*/0,
       /*Flags=*/static_cast<llvm::DINode::DIFlags>(attr.getFlags()),
       llvm::MDNode::get(llvmCtx, elements),
@@ -126,9 +136,6 @@ DebugTranslation::translateImpl(DICompositeTypeAttr attr) {
 }
 
 llvm::DIDerivedType *DebugTranslation::translateImpl(DIDerivedTypeAttr attr) {
-  auto getMDStringOrNull = [&](StringAttr attr) -> llvm::MDString * {
-    return attr ? llvm::MDString::get(llvmCtx, attr) : nullptr;
-  };
   return llvm::DIDerivedType::get(
       llvmCtx, attr.getTag(), getMDStringOrNull(attr.getName()),
       /*File=*/nullptr, /*Line=*/0,
@@ -138,7 +145,8 @@ llvm::DIDerivedType *DebugTranslation::translateImpl(DIDerivedTypeAttr attr) {
 }
 
 llvm::DIFile *DebugTranslation::translateImpl(DIFileAttr attr) {
-  return llvm::DIFile::get(llvmCtx, attr.getName(), attr.getDirectory());
+  return llvm::DIFile::get(llvmCtx, getMDStringOrNull(attr.getName()),
+                           getMDStringOrNull(attr.getDirectory()));
 }
 
 llvm::DILexicalBlock *DebugTranslation::translateImpl(DILexicalBlockAttr attr) {
@@ -161,9 +169,9 @@ llvm::DILocalScope *DebugTranslation::translateImpl(DILocalScopeAttr attr) {
 llvm::DILocalVariable *
 DebugTranslation::translateImpl(DILocalVariableAttr attr) {
   return llvm::DILocalVariable::get(
-      llvmCtx, translate(attr.getScope()),
-      llvm::MDString::get(llvmCtx, attr.getName()), translate(attr.getFile()),
-      attr.getLine(), translate(attr.getType()), attr.getArg(),
+      llvmCtx, translate(attr.getScope()), getMDStringOrNull(attr.getName()),
+      translate(attr.getFile()), attr.getLine(), translate(attr.getType()),
+      attr.getArg(),
       /*Flags=*/llvm::DINode::FlagZero, attr.getAlignInBits(),
       /*Annotations=*/nullptr);
 }
@@ -184,12 +192,9 @@ static llvm::DISubprogram *getSubprogram(bool isDistinct, Ts &&...args) {
 llvm::DISubprogram *DebugTranslation::translateImpl(DISubprogramAttr attr) {
   bool isDefinition = static_cast<bool>(attr.getSubprogramFlags() &
                                         LLVM::DISubprogramFlags::Definition);
-  auto getMDStringOrNull = [&](StringAttr attr) -> llvm::MDString * {
-    return attr ? llvm::MDString::get(llvmCtx, attr) : nullptr;
-  };
   return getSubprogram(
       isDefinition, llvmCtx, translate(attr.getScope()),
-      llvm::MDString::get(llvmCtx, attr.getName()),
+      getMDStringOrNull(attr.getName()),
       getMDStringOrNull(attr.getLinkageName()), translate(attr.getFile()),
       attr.getLine(), translate(attr.getType()), attr.getScopeLine(),
       /*ContainingType=*/nullptr, /*VirtualIndex=*/0,
@@ -200,7 +205,8 @@ llvm::DISubprogram *DebugTranslation::translateImpl(DISubprogramAttr attr) {
 
 llvm::DINamespace *DebugTranslation::translateImpl(DINamespaceAttr attr) {
   return llvm::DINamespace::get(llvmCtx, translate(attr.getScope()),
-                                attr.getName(), attr.getExportSymbols());
+                                getMDStringOrNull(attr.getName()),
+                                attr.getExportSymbols());
 }
 
 llvm::DISubrange *DebugTranslation::translateImpl(DISubrangeAttr attr) {

diff  --git a/mlir/lib/Target/LLVMIR/DebugTranslation.h b/mlir/lib/Target/LLVMIR/DebugTranslation.h
index 4160ca09a1c90..3ea077a80bfa1 100644
--- a/mlir/lib/Target/LLVMIR/DebugTranslation.h
+++ b/mlir/lib/Target/LLVMIR/DebugTranslation.h
@@ -78,6 +78,10 @@ class DebugTranslation {
   llvm::DISubroutineType *translateImpl(DISubroutineTypeAttr attr);
   llvm::DIType *translateImpl(DITypeAttr attr);
 
+  /// Constructs a string metadata node from the string attribute. Returns
+  /// nullptr if `stringAttr` is null or contains and empty string.
+  llvm::MDString *getMDStringOrNull(StringAttr stringAttr);
+
   /// A mapping between mlir location+scope and the corresponding llvm debug
   /// metadata.
   DenseMap<std::tuple<Location, llvm::DILocalScope *, const llvm::DILocation *>,

diff  --git a/mlir/test/Target/LLVMIR/Import/debug-info.ll b/mlir/test/Target/LLVMIR/Import/debug-info.ll
index fa1dccb471798..a8aae4573e9af 100644
--- a/mlir/test/Target/LLVMIR/Import/debug-info.ll
+++ b/mlir/test/Target/LLVMIR/Import/debug-info.ll
@@ -190,7 +190,7 @@ define void @composite_type() !dbg !3 {
 ; // -----
 
 ; CHECK-DAG: #[[FILE:.+]] = #llvm.di_file<"debug-info.ll" in "/">
-; CHECK-DAG: #[[CU:.+]] = #llvm.di_compile_unit<sourceLanguage = DW_LANG_C, file = #[[FILE]], producer = "", isOptimized = false, emissionKind = None>
+; CHECK-DAG: #[[CU:.+]] = #llvm.di_compile_unit<sourceLanguage = DW_LANG_C, file = #[[FILE]], isOptimized = false, emissionKind = None>
 ; Verify an empty subroutine types list is supported.
 ; CHECK-DAG: #[[SP_TYPE:.+]] = #llvm.di_subroutine_type<callingConvention = DW_CC_normal>
 ; CHECK-DAG: #[[SP:.+]] = #llvm.di_subprogram<compileUnit = #[[CU]], scope = #[[FILE]], name = "subprogram", linkageName = "subprogram", file = #[[FILE]], line = 42, scopeLine = 42, subprogramFlags = Definition, type = #[[SP_TYPE]]>
@@ -389,3 +389,24 @@ declare void @llvm.dbg.value(metadata, metadata, metadata)
 !8 = distinct !DISubprogram(name: "namespace", scope: !10, file: !2, unit: !1);
 !9 = !DILocation(line: 1, column: 2, scope: !8)
 !10 = !DINamespace(name: "std", scope: null)
+
+; // -----
+
+; CHECK-DAG: #[[SUBPROGRAM:.+]] =  #llvm.di_subprogram<compileUnit = #{{.*}}, scope = #{{.*}}, name = "noname_variable"
+; CHECK-DAG: #[[LOCAL_VARIABLE:.+]] =  #llvm.di_local_variable<scope = #[[SUBPROGRAM]]>
+
+define void @noname_variable(ptr %arg) {
+  call void @llvm.dbg.value(metadata ptr %arg, metadata !7, metadata !DIExpression()), !dbg !9
+  ret void
+}
+
+declare void @llvm.dbg.value(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!1}
+!llvm.module.flags = !{!0}
+!0 = !{i32 2, !"Debug Info Version", i32 3}
+!1 = distinct !DICompileUnit(language: DW_LANG_C, file: !2)
+!2 = !DIFile(filename: "debug-info.ll", directory: "/")
+!7 = !DILocalVariable(scope: !8)
+!8 = distinct !DISubprogram(name: "noname_variable", scope: !2, file: !2, unit: !1);
+!9 = !DILocation(line: 1, column: 2, scope: !8)

diff  --git a/mlir/test/Target/LLVMIR/llvmir-debug.mlir b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
index 6e07b685abd9e..a628349951e60 100644
--- a/mlir/test/Target/LLVMIR/llvmir-debug.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
@@ -72,6 +72,7 @@ llvm.func @func_no_debug() {
 #blockScope = #llvm.di_lexical_block<scope = #sp0>
 #variable = #llvm.di_local_variable<scope = #fileScope, name = "arg", file = #file, line = 6, arg = 1, alignInBits = 32, type = #si64>
 #variableAddr = #llvm.di_local_variable<scope = #blockScope, name = "alloc">
+#noNameVariable = #llvm.di_local_variable<scope = #blockScope>
 
 #spType1 = #llvm.di_subroutine_type<callingConvention = DW_CC_normal>
 #sp1 = #llvm.di_subprogram<
@@ -89,9 +90,11 @@ llvm.func @func_with_debug(%arg: i64) {
   // CHECK: call void @llvm.dbg.value(metadata i64 %[[ARG]], metadata ![[VAR_LOC:[0-9]+]], metadata !DIExpression())
   // CHECK: call void @llvm.dbg.addr(metadata ptr %[[ALLOC]], metadata ![[ADDR_LOC:[0-9]+]], metadata !DIExpression())
   // CHECK: call void @llvm.dbg.declare(metadata ptr %[[ALLOC]], metadata ![[ADDR_LOC]], metadata !DIExpression())
+  // CHECK: call void @llvm.dbg.value(metadata i64 %[[ARG]], metadata ![[NO_NAME_VAR:[0-9]+]], metadata !DIExpression())
   llvm.intr.dbg.value #variable = %arg : i64
   llvm.intr.dbg.addr #variableAddr = %alloc : !llvm.ptr<i64>
   llvm.intr.dbg.declare #variableAddr = %alloc : !llvm.ptr<i64>
+  llvm.intr.dbg.value #noNameVariable= %arg : i64
 
   // CHECK: call void @func_no_debug(), !dbg ![[CALLSITE_LOC:[0-9]+]]
   llvm.call @func_no_debug() : () -> () loc(callsite("mysource.cc":3:4 at "mysource.cc":5:6))
@@ -139,6 +142,7 @@ llvm.func @empty_types() {
 // CHECK: ![[VAR_SCOPE]] = distinct !DILexicalBlockFile(scope: ![[FUNC_LOC]], file: ![[CU_FILE_LOC]], discriminator: 0)
 // CHECK: ![[ADDR_LOC]] = !DILocalVariable(name: "alloc", scope: ![[BLOCK_LOC:.*]])
 // CHECK: ![[BLOCK_LOC]] = distinct !DILexicalBlock(scope: ![[FUNC_LOC]])
+// CHECK: ![[NO_NAME_VAR]] = !DILocalVariable(scope: ![[BLOCK_LOC]])
 
 // CHECK-DAG: ![[CALLSITE_LOC]] = !DILocation(line: 3, column: 4,
 // CHECK-DAG: ![[FILE_LOC]] = !DILocation(line: 1, column: 2,


        


More information about the Mlir-commits mailing list