-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[MLIR][LLVM] Extend DIScopeForLLVMFuncOp to handle cross-file operatio… #167844
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][LLVM] Extend DIScopeForLLVMFuncOp to handle cross-file operatio… #167844
Conversation
|
@llvm/pr-subscribers-mlir-llvm @llvm/pr-subscribers-mlir Author: Zichen Lu (MikaOvO) ChangesThe current Scenario: # a.py
@<!-- -->cute.kernel
def kernel_a(tensor):
cute.print("a: {}", tensor) # a.py:3
jit_func_b(tensor) # Calls b.py code
# b.py
@<!-- -->cute.jit
def jit_func_b(tensor):
cute.print("b: {}", tensor) # b.py:7The DSL executes Python at compile-time and directly inserts operations from cuda.kernel @<!-- -->kernel_a(...) {
cute.print("a: {}", %arg0) loc(#loc_a) // a.py:3
cute.print("b: {}", %arg0) loc(#loc_b) // b.py:7 <- FileLineColLoc, not CallSiteLoc
} loc(#loc_kernel) // a.py:1
#loc1 = loc("a.py":3:.)
#loc2 = loc("b.py":7:.)
#loc_a = loc("cute.printf"(#loc1))
#loc_b = loc("cute.printf"(#loc2))!6 = !DIFile(filename: "a.py", directory: "...")
!9 = distinct !DISubprogram(name: "...", linkageName: "...", scope: !6, file: !6, line: 13, ...)
!10 = !DILocation(line: 7, column: ., scope: !9) // Points to kernel's DISubprogram, not correctFull diff: https://github.com/llvm/llvm-project/pull/167844.diff 1 Files Affected:
diff --git a/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp b/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp
index 67573c4ee6061..8862cf8e70366 100644
--- a/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp
+++ b/mlir/lib/Dialect/LLVMIR/Transforms/DIScopeForLLVMFuncOp.cpp
@@ -122,7 +122,44 @@ static void setLexicalBlockFileAttr(Operation *op) {
op->setLoc(
CallSiteLoc::get(getNestedLoc(op, scopeAttr, calleeLoc), callerLoc));
}
+
+ return;
}
+
+ auto funcOp = op->getParentOfType<LLVM::LLVMFuncOp>();
+ if (!funcOp)
+ return;
+
+ FileLineColLoc opFileLoc = extractFileLoc(opLoc);
+ if (!opFileLoc)
+ return;
+
+ FileLineColLoc funcFileLoc = extractFileLoc(funcOp.getLoc());
+ if (!funcFileLoc)
+ return;
+
+ StringRef opFile = opFileLoc.getFilename().getValue();
+ StringRef funcFile = funcFileLoc.getFilename().getValue();
+
+ if (opFile != funcFile) {
+ auto funcOpLoc = llvm::dyn_cast_if_present<FusedLoc>(funcOp.getLoc());
+ if (!funcOpLoc)
+ return;
+ auto scopeAttr = dyn_cast<LLVM::DISubprogramAttr>(funcOpLoc.getMetadata());
+ if (!scopeAttr)
+ return;
+
+ auto *context = op->getContext();
+ LLVM::DIFileAttr opFileAttr =
+ LLVM::DIFileAttr::get(context, llvm::sys::path::filename(opFile),
+ llvm::sys::path::parent_path(opFile));
+
+ auto lexicalBlockFileAttr = LLVM::DILexicalBlockFileAttr::get(
+ context, scopeAttr, opFileAttr, 0);
+
+ Location newLoc = FusedLoc::get(context, {opLoc}, lexicalBlockFileAttr);
+ op->setLoc(newLoc);
+ }
}
namespace {
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
49cd321 to
5459b85
Compare
|
@joker-eph @grypp Could you help review it? Thanks! |
| LLVM::DIFileAttr::get(context, llvm::sys::path::filename(opFile), | ||
| llvm::sys::path::parent_path(opFile)); | ||
|
|
||
| auto lexicalBlockFileAttr = |
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.
LLVM codebase doesn't use auto when the type is not obvious.
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.
Updated it, thanks!
| @@ -110,7 +110,9 @@ static Location getNestedLoc(Operation *op, LLVM::DIScopeAttr scopeAttr, | |||
| } | |||
|
|
|||
| static void setLexicalBlockFileAttr(Operation *op) { | |||
| if (auto callSiteLoc = dyn_cast<CallSiteLoc>(op->getLoc())) { | |||
| Location opLoc = op->getLoc(); | |||
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.
how can we test this code?
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.
If this change is accepted, I plan to add a simple MLIR file generated from merging multiple source files and check whether the location information in the results includes multiple files.
7fcd906 to
9a8a3f8
Compare
🐧 Linux x64 Test Results
|
0b55446 to
c6e331b
Compare
| LLVM::DILexicalBlockFileAttr::get(context, scopeAttr, opFileAttr, 0); | ||
|
|
||
| Location newLoc = FusedLoc::get(context, {opLoc}, lexicalBlockFileAttr); | ||
| op->setLoc(newLoc); |
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 some high-level documentation before this new code block? I would document the logic at the function level I think.
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.
Updated, thanks!
…ons without CallSiteLoc
ae477ee to
ba373a4
Compare
The current
DIScopeForLLVMFuncOppass handles debug information for inlined code by processingCallSiteLocattributes. However, some compilation scenarios compose code from multiple source files directly into a single function without generatingCallSiteLoc.Scenario:
The scenario executes Python at compile-time and directly inserts operations from
b.pyinto the kernel function, resulting in MLIR like: