Skip to content

[mlir][gpu] Fix bug with gpu.printf global location #142872

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

Merged
merged 3 commits into from
Jun 5, 2025

Conversation

adstraw
Copy link
Contributor

@adstraw adstraw commented Jun 4, 2025

Bug description: Global variables and functions created during gpu.printf conversion to NVVM may contain debug info metadata from function containing the gpu.printf which cannot be used out of that function.

@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir:gpu mlir labels Jun 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 4, 2025

@llvm/pr-subscribers-mlir-core
@llvm/pr-subscribers-mlir-gpu

@llvm/pr-subscribers-mlir

Author: Adam Straw (adstraw)

Changes

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

3 Files Affected:

  • (modified) mlir/include/mlir/IR/Location.h (+10)
  • (modified) mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp (+10-4)
  • (added) mlir/test/Conversion/GPUToNVVM/gpu-to-nvvm-debuginfo.mlir (+24)
diff --git a/mlir/include/mlir/IR/Location.h b/mlir/include/mlir/IR/Location.h
index 5b1cf300295e5..d286befe1920c 100644
--- a/mlir/include/mlir/IR/Location.h
+++ b/mlir/include/mlir/IR/Location.h
@@ -21,6 +21,7 @@ namespace mlir {
 
 class Location;
 class WalkResult;
+class UnknownLoc;
 
 //===----------------------------------------------------------------------===//
 // LocationAttr
@@ -53,6 +54,15 @@ class LocationAttr : public Attribute {
     return result;
   }
 
+  /// Return an instance of the given location type if one is nested under the
+  /// current location else return unknown location.
+  template <typename T, typename UnknownT = UnknownLoc>
+  LocationAttr findInstanceOfOrUnknown() {
+    if (T result = findInstanceOf<T>())
+      return result;
+    return UnknownT::get(getContext());
+  }
+
   /// Methods for support type inquiry through isa, cast, and dyn_cast.
   static bool classof(Attribute attr);
 };
diff --git a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
index dea3d99704d9b..609f549a9f009 100644
--- a/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
+++ b/mlir/lib/Conversion/GPUCommon/GPUOpsLowering.cpp
@@ -543,14 +543,20 @@ LogicalResult GPUPrintfOpToVPrintfLowering::matchAndRewrite(
   // the device code, not the host code
   auto moduleOp = gpuPrintfOp->getParentOfType<gpu::GPUModuleOp>();
 
+  // Convert the location to a valid global location of type FileLineColLoc if
+  // found else UnknownLoc. Remove any metadata from the location which is not
+  // valid for a global location.
+  Location globalLoc = loc->findInstanceOfOrUnknown<FileLineColLoc>();
+
   auto vprintfType =
       LLVM::LLVMFunctionType::get(rewriter.getI32Type(), {ptrType, ptrType});
-  LLVM::LLVMFuncOp vprintfDecl =
-      getOrDefineFunction(moduleOp, loc, rewriter, "vprintf", vprintfType);
+  LLVM::LLVMFuncOp vprintfDecl = getOrDefineFunction(
+      moduleOp, globalLoc, rewriter, "vprintf", vprintfType);
 
   // Create the global op or find an existing one.
-  LLVM::GlobalOp global = getOrCreateStringConstant(
-      rewriter, loc, moduleOp, llvmI8, "printfFormat_", adaptor.getFormat());
+  LLVM::GlobalOp global =
+      getOrCreateStringConstant(rewriter, globalLoc, moduleOp, llvmI8,
+                                "printfFormat_", adaptor.getFormat());
 
   // Get a pointer to the format string's first element
   Value globalPtr = rewriter.create<LLVM::AddressOfOp>(loc, global);
diff --git a/mlir/test/Conversion/GPUToNVVM/gpu-to-nvvm-debuginfo.mlir b/mlir/test/Conversion/GPUToNVVM/gpu-to-nvvm-debuginfo.mlir
new file mode 100644
index 0000000000000..66cac733785b3
--- /dev/null
+++ b/mlir/test/Conversion/GPUToNVVM/gpu-to-nvvm-debuginfo.mlir
@@ -0,0 +1,24 @@
+// RUN: mlir-opt %s -convert-gpu-to-nvvm='has-redux=1' -mlir-print-debuginfo | FileCheck %s
+
+#di_file = #llvm.di_file<"foo.mlir" in "/tmp/">
+#di_compile_unit = #llvm.di_compile_unit<
+  id = distinct[0]<>, sourceLanguage = DW_LANG_C, file = #di_file,
+  producer = "MLIR", isOptimized = true, emissionKind = Full
+>
+#di_subprogram = #llvm.di_subprogram<
+  compileUnit = #di_compile_unit, scope = #di_file, name = "test_const_printf_with_loc",
+  file = #di_file, subprogramFlags = "Definition"
+>
+
+// CHECK-DAG: [[LOC:#[a-zA-Z0-9_]+]] = loc("foo.mlir":0:0)
+#loc = loc("foo.mlir":0:0)
+
+gpu.module @test_module_56 {
+  // CHECK-DAG: llvm.mlir.global internal constant @[[$PRINT_GLOBAL0:[A-Za-z0-9_]+]]("Hello, world with location\0A\00") {addr_space = 0 : i32} loc([[LOC]])
+  // CHECK-DAG: llvm.func @vprintf(!llvm.ptr, !llvm.ptr) -> i32 loc([[LOC]])
+
+  gpu.func @test_const_printf_with_loc() {
+    gpu.printf "Hello, world with location\n" loc(fused<#di_subprogram>[#loc])
+    gpu.return
+  }
+}

@adstraw
Copy link
Contributor Author

adstraw commented Jun 4, 2025

@River707 please review

Copy link
Contributor

@River707 River707 left a comment

Choose a reason for hiding this comment

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

Can you add more context to the description on what the "bug" is here?

Copy link
Contributor

@River707 River707 left a comment

Choose a reason for hiding this comment

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

LGTM, please update the MR description with more details on what the issue is here.

@River707 River707 merged commit 3172c61 into llvm:main Jun 5, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlir:core MLIR Core Infrastructure mlir:gpu mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants