-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[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
Conversation
@llvm/pr-subscribers-mlir-core @llvm/pr-subscribers-mlir Author: Adam Straw (adstraw) ChangesFull diff: https://github.com/llvm/llvm-project/pull/142872.diff 3 Files Affected:
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
+ }
+}
|
@River707 please review |
There was a problem hiding this 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?
There was a problem hiding this 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.
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.