[mlir] Allow fallback from file line col range to loc #124321
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This was discussed during the original review but I made it stricter than discussed. Making it a pure view but adding a helper for bytecode serialization.
|
@llvm/pr-subscribers-mlir-core @llvm/pr-subscribers-mlir-llvm Author: Jacques Pienaar (jpienaar) ChangesThis was discussed during the original review but I made it stricter than discussed. Making it a pure view but adding a helper for bytecode serialization (I could avoid the helper, but it ends up with more logic and stronger coupling). Full diff: https://github.com/llvm/llvm-project/pull/124321.diff 4 Files Affected:
diff --git a/mlir/include/mlir/IR/BuiltinDialectBytecode.td b/mlir/include/mlir/IR/BuiltinDialectBytecode.td
index 87da8fd3568fa2..0208e8cdbf2931 100644
--- a/mlir/include/mlir/IR/BuiltinDialectBytecode.td
+++ b/mlir/include/mlir/IR/BuiltinDialectBytecode.td
@@ -104,7 +104,7 @@ def FileLineColRange : DialectAttribute<(attr
WithPrinter<"writeFileLineColRangeLocs($_writer, $_name)">>>>:$rawLocData
)> {
let cBuilder = "getFileLineColRange(context, filename, rawLocData)";
- let printerPredicate = "!::llvm::isa<FileLineColLoc>($_val)";
+ let printerPredicate = "!isStrictFileLineColLoc($_val)";
}
def FileLineColLoc : DialectAttribute<(attr
@@ -112,7 +112,7 @@ def FileLineColLoc : DialectAttribute<(attr
VarInt:$start_line,
VarInt:$start_column
)> {
- let printerPredicate = "::llvm::isa<FileLineColLoc>($_val)";
+ let printerPredicate = "isStrictFileLineColLoc($_val)";
}
}
diff --git a/mlir/include/mlir/IR/Location.h b/mlir/include/mlir/IR/Location.h
index e206501f5ee6a2..8ce36ed415ac1b 100644
--- a/mlir/include/mlir/IR/Location.h
+++ b/mlir/include/mlir/IR/Location.h
@@ -177,7 +177,7 @@ class FusedLocWith : public FusedLoc {
/// column number. This is similar to the type of location that you get from
/// most source languages.
///
-/// FileLineColLoc is a FileLineColRange with exactly one line and column.
+/// FileLineColLoc is a view to FileLineColRange with one line and column.
class FileLineColLoc : public FileLineColRange {
public:
using FileLineColRange::FileLineColRange;
@@ -190,11 +190,12 @@ class FileLineColLoc : public FileLineColRange {
StringAttr getFilename() const;
unsigned getLine() const;
unsigned getColumn() const;
-
- /// Methods for support type inquiry through isa, cast, and dyn_cast.
- static bool classof(Attribute attr);
};
+/// Returns true iff the given location is a FileLineColRange with exactly one
+/// line and column.
+bool isStrictFileLineColLoc(Location loc);
+
//===----------------------------------------------------------------------===//
// OpaqueLoc
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/IR/Location.cpp b/mlir/lib/IR/Location.cpp
index ce78d30ee0a526..7a4df4fbd46d9c 100644
--- a/mlir/lib/IR/Location.cpp
+++ b/mlir/lib/IR/Location.cpp
@@ -177,10 +177,8 @@ unsigned FileLineColLoc::getLine() const { return getStartLine(); }
unsigned FileLineColLoc::getColumn() const { return getStartColumn(); }
-bool FileLineColLoc::classof(Attribute attr) {
- // This could also have been for <= 2. But given this is matching previous
- // behavior, it is left as is.
- if (auto range = mlir::dyn_cast<FileLineColRange>(attr))
+bool mlir::isStrictFileLineColLoc(Location loc) {
+ if (auto range = mlir::dyn_cast<FileLineColRange>(loc))
return range.getImpl()->size() == 2;
return false;
}
diff --git a/mlir/test/Target/LLVMIR/llvmir-debug.mlir b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
index eac2c5090a5b5c..d15274311d7451 100644
--- a/mlir/test/Target/LLVMIR/llvmir-debug.mlir
+++ b/mlir/test/Target/LLVMIR/llvmir-debug.mlir
@@ -115,6 +115,9 @@ llvm.func @func_with_debug(%arg: i64) {
// CHECK: call void @func_no_debug(), !dbg ![[FILE_LOC:[0-9]+]]
llvm.call @func_no_debug() : () -> () loc("foo.mlir":1:2)
+ // CHECK: call void @func_no_debug(), !dbg ![[FILE_LOC:[0-9]+]]
+ llvm.call @func_no_debug() : () -> () loc("foo.mlir":1:2 to 5:6)
+
// CHECK: call void @func_no_debug(), !dbg ![[NAMED_LOC:[0-9]+]]
llvm.call @func_no_debug() : () -> () loc("named"("foo.mlir":10:10))
|
River707
approved these changes
Jan 24, 2025
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/50/builds/9391 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was discussed during the original review but I made it stricter than discussed. Making it a pure view but adding a helper for bytecode serialization (I could avoid the helper, but it ends up with more logic and stronger coupling).