[Mlir-commits] [mlir] 892605b - [mlir][Asm] Add support for using an alias for trailing operation locations
River Riddle
llvmlistbot at llvm.org
Mon Nov 9 22:03:37 PST 2020
Author: River Riddle
Date: 2020-11-09T21:54:47-08:00
New Revision: 892605b449f8375c62227df2eac2b8f1d7180af6
URL: https://github.com/llvm/llvm-project/commit/892605b449f8375c62227df2eac2b8f1d7180af6
DIFF: https://github.com/llvm/llvm-project/commit/892605b449f8375c62227df2eac2b8f1d7180af6.diff
LOG: [mlir][Asm] Add support for using an alias for trailing operation locations
Locations often get very long and clutter up operations when printed inline with them. This revision adds support for using aliases with trailing operation locations, and makes printing with aliases the default behavior. Aliases in the trailing location take the form `loc(<alias>)`, such as `loc(#loc0)`. As with all aliases, using `mlir-print-local-scope` can be used to disable them and get the inline behavior.
Differential Revision: https://reviews.llvm.org/D90652
Added:
Modified:
mlir/lib/IR/AsmPrinter.cpp
mlir/lib/IR/MLIRContext.cpp
mlir/lib/Parser/AttributeParser.cpp
mlir/lib/Parser/LocationParser.cpp
mlir/lib/Parser/Parser.h
mlir/test/Dialect/SPIRV/Serialization/debug.mlir
mlir/test/IR/invalid-locations.mlir
mlir/test/IR/locations.mlir
mlir/test/IR/module-op.mlir
mlir/test/IR/opaque_locations.mlir
mlir/test/IR/parser.mlir
mlir/test/IR/wrapping_op.mlir
mlir/test/Transforms/inlining.mlir
mlir/test/Transforms/location-snapshot.mlir
mlir/test/Transforms/strip-debuginfo.mlir
mlir/test/mlir-tblgen/pattern.mlir
Removed:
################################################################################
diff --git a/mlir/lib/IR/AsmPrinter.cpp b/mlir/lib/IR/AsmPrinter.cpp
index ff6d8ffe653f..4b91b19c5d9f 100644
--- a/mlir/lib/IR/AsmPrinter.cpp
+++ b/mlir/lib/IR/AsmPrinter.cpp
@@ -289,7 +289,9 @@ class DummyAliasOperationPrinter : private OpAsmPrinter {
/// Print the given operation.
void print(Operation *op) {
- // TODO: Consider the operation location for an alias.
+ // Visit the operation location.
+ if (printerFlags.shouldPrintDebugInfo())
+ initializer.visit(op->getLoc());
// If requested, always print the generic form.
if (!printerFlags.shouldPrintGenericOpForm()) {
@@ -1089,7 +1091,10 @@ class ModulePrinter {
AttrTypeElision typeElision = AttrTypeElision::Never);
void printType(Type type);
- void printLocation(LocationAttr loc);
+
+ /// Print the given location to the stream. If `allowAlias` is true, this
+ /// allows for the internal location to use an attribute alias.
+ void printLocation(LocationAttr loc, bool allowAlias = false);
void printAffineMap(AffineMap map);
void
@@ -1152,7 +1157,7 @@ void ModulePrinter::printTrailingLocation(Location loc) {
return;
os << " ";
- printLocation(loc);
+ printLocation(loc, /*allowAlias=*/true);
}
void ModulePrinter::printLocationInternal(LocationAttr loc, bool pretty) {
@@ -1269,14 +1274,14 @@ static void printFloatValue(const APFloat &apValue, raw_ostream &os) {
os << str;
}
-void ModulePrinter::printLocation(LocationAttr loc) {
- if (printerFlags.shouldPrintDebugInfoPrettyForm()) {
- printLocationInternal(loc, /*pretty=*/true);
- } else {
- os << "loc(";
+void ModulePrinter::printLocation(LocationAttr loc, bool allowAlias) {
+ if (printerFlags.shouldPrintDebugInfoPrettyForm())
+ return printLocationInternal(loc, /*pretty=*/true);
+
+ os << "loc(";
+ if (!allowAlias || !state || failed(state->getAliasState().getAlias(loc, os)))
printLocationInternal(loc);
- os << ')';
- }
+ os << ')';
}
/// Returns true if the given dialect symbol data is simple enough to print in
diff --git a/mlir/lib/IR/MLIRContext.cpp b/mlir/lib/IR/MLIRContext.cpp
index 284cf2a67b79..cdebb2a5a8d0 100644
--- a/mlir/lib/IR/MLIRContext.cpp
+++ b/mlir/lib/IR/MLIRContext.cpp
@@ -99,6 +99,10 @@ struct BuiltinOpAsmDialectInterface : public OpAsmDialectInterface {
os << "set";
return success();
}
+ if (attr.isa<LocationAttr>()) {
+ os << "loc";
+ return success();
+ }
return failure();
}
};
diff --git a/mlir/lib/Parser/AttributeParser.cpp b/mlir/lib/Parser/AttributeParser.cpp
index e8f5213768bd..6edc98d145c6 100644
--- a/mlir/lib/Parser/AttributeParser.cpp
+++ b/mlir/lib/Parser/AttributeParser.cpp
@@ -121,8 +121,14 @@ Attribute Parser::parseAttribute(Type type) {
// Parse a location attribute.
case Token::kw_loc: {
- LocationAttr attr;
- return failed(parseLocation(attr)) ? Attribute() : attr;
+ consumeToken(Token::kw_loc);
+
+ LocationAttr locAttr;
+ if (parseToken(Token::l_paren, "expected '(' in inline location") ||
+ parseLocationInstance(locAttr) ||
+ parseToken(Token::r_paren, "expected ')' in inline location"))
+ return Attribute();
+ return locAttr;
}
// Parse an opaque elements attribute.
diff --git a/mlir/lib/Parser/LocationParser.cpp b/mlir/lib/Parser/LocationParser.cpp
index 886514a92492..7ad8b92d354b 100644
--- a/mlir/lib/Parser/LocationParser.cpp
+++ b/mlir/lib/Parser/LocationParser.cpp
@@ -11,24 +11,6 @@
using namespace mlir;
using namespace mlir::detail;
-/// Parse a location.
-///
-/// location ::= `loc` inline-location
-/// inline-location ::= '(' location-inst ')'
-///
-ParseResult Parser::parseLocation(LocationAttr &loc) {
- // Check for 'loc' identifier.
- if (parseToken(Token::kw_loc, "expected 'loc' keyword"))
- return emitError();
-
- // Parse the inline-location.
- if (parseToken(Token::l_paren, "expected '(' in inline location") ||
- parseLocationInstance(loc) ||
- parseToken(Token::r_paren, "expected ')' in inline location"))
- return failure();
- return success();
-}
-
/// Specific location instances.
///
/// location-inst ::= filelinecol-location |
@@ -195,3 +177,35 @@ ParseResult Parser::parseLocationInstance(LocationAttr &loc) {
return emitError("expected location instance");
}
+
+ParseResult Parser::parseOptionalTrailingLocation(Location &loc) {
+ // If there is a 'loc' we parse a trailing location.
+ if (!consumeIf(Token::kw_loc))
+ return success();
+ if (parseToken(Token::l_paren, "expected '(' in location"))
+ return failure();
+ Token tok = getToken();
+
+ // Check to see if we are parsing a location alias.
+ LocationAttr directLoc;
+ if (tok.is(Token::hash_identifier)) {
+ // TODO: This should be reworked a bit to allow for resolving operation
+ // locations to aliases after the operation has already been parsed(i.e.
+ // allow post parse location fixups).
+ Attribute attr = parseExtendedAttr(Type());
+ if (!attr)
+ return failure();
+ if (!(directLoc = attr.dyn_cast<LocationAttr>()))
+ return emitError(tok.getLoc()) << "expected location, but found " << attr;
+
+ // Otherwise, we parse the location directly.
+ } else if (parseLocationInstance(directLoc)) {
+ return failure();
+ }
+
+ if (parseToken(Token::r_paren, "expected ')' in location"))
+ return failure();
+
+ loc = directLoc;
+ return success();
+}
diff --git a/mlir/lib/Parser/Parser.h b/mlir/lib/Parser/Parser.h
index 63a9d0df040c..bec9cf5d04c9 100644
--- a/mlir/lib/Parser/Parser.h
+++ b/mlir/lib/Parser/Parser.h
@@ -231,9 +231,6 @@ class Parser {
// Location Parsing
//===--------------------------------------------------------------------===//
- /// Parse an inline location.
- ParseResult parseLocation(LocationAttr &loc);
-
/// Parse a raw location instance.
ParseResult parseLocationInstance(LocationAttr &loc);
@@ -248,20 +245,9 @@ class Parser {
/// Parse an optional trailing location.
///
- /// trailing-location ::= (`loc` `(` location `)`)?
+ /// trailing-location ::= (`loc` (`(` location `)` | attribute-alias))?
///
- ParseResult parseOptionalTrailingLocation(Location &loc) {
- // If there is a 'loc' we parse a trailing location.
- if (!getToken().is(Token::kw_loc))
- return success();
-
- // Parse the location.
- LocationAttr directLoc;
- if (parseLocation(directLoc))
- return failure();
- loc = directLoc;
- return success();
- }
+ ParseResult parseOptionalTrailingLocation(Location &loc);
//===--------------------------------------------------------------------===//
// Affine Parsing
diff --git a/mlir/test/Dialect/SPIRV/Serialization/debug.mlir b/mlir/test/Dialect/SPIRV/Serialization/debug.mlir
index d5c5ca6a351c..a6b8ed3bc71c 100644
--- a/mlir/test/Dialect/SPIRV/Serialization/debug.mlir
+++ b/mlir/test/Dialect/SPIRV/Serialization/debug.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-translate -test-spirv-roundtrip-debug -mlir-print-debuginfo %s | FileCheck %s
+// RUN: mlir-translate -test-spirv-roundtrip-debug -mlir-print-debuginfo -mlir-print-local-scope %s | FileCheck %s
spv.module Logical GLSL450 requires #spv.vce<v1.0, [Shader], []> {
// CHECK: loc({{".*debug.mlir"}}:5:3)
diff --git a/mlir/test/IR/invalid-locations.mlir b/mlir/test/IR/invalid-locations.mlir
index 417771638716..06f7951f5c34 100644
--- a/mlir/test/IR/invalid-locations.mlir
+++ b/mlir/test/IR/invalid-locations.mlir
@@ -4,14 +4,14 @@
func @location_missing_l_paren() {
^bb:
- return loc) // expected-error {{expected '(' in inline location}}
+ return loc) // expected-error {{expected '(' in location}}
}
// -----
func @location_missing_r_paren() {
^bb:
- return loc(unknown // expected-error at +1 {{expected ')' in inline location}}
+ return loc(unknown // expected-error at +1 {{expected ')' in location}}
}
// -----
@@ -98,3 +98,11 @@ func @location_fused_missing_r_square() {
^bb:
return loc(fused[unknown) // expected-error {{expected ']' in fused location}}
}
+
+// -----
+
+func @location_invalid_alias() {
+ // expected-error at +1 {{expected location, but found #foo.loc}}
+ return loc(#foo.loc)
+}
+
diff --git a/mlir/test/IR/locations.mlir b/mlir/test/IR/locations.mlir
index 6227ba59e6ec..1a8387187443 100644
--- a/mlir/test/IR/locations.mlir
+++ b/mlir/test/IR/locations.mlir
@@ -1,4 +1,5 @@
-// RUN: mlir-opt -allow-unregistered-dialect %s -mlir-print-debuginfo | FileCheck %s
+// RUN: mlir-opt -allow-unregistered-dialect %s -mlir-print-debuginfo -mlir-print-local-scope | FileCheck %s
+// RUN: mlir-opt -allow-unregistered-dialect %s -mlir-print-debuginfo | FileCheck %s --check-prefix=CHECK-ALIAS
// This test verifies that debug locations are round-trippable.
#set0 = affine_set<(d0) : (1 == 0)>
@@ -22,3 +23,12 @@ func @inline_notation() -> i32 {
// CHECK: return %0 : i32 loc(unknown)
return %1 : i32 loc(unknown)
}
+
+// CHECK-LABEL: func @loc_attr(i1 {foo.loc_attr = loc(callsite("foo" at "mysource.cc":10:8))})
+func @loc_attr(i1 {foo.loc_attr = loc(callsite("foo" at "mysource.cc":10:8))})
+
+// CHECK-ALIAS: #[[LOC:.*]] = loc("out_of_line_location")
+#loc = loc("out_of_line_location")
+
+// CHECK-ALIAS: "foo.op"() : () -> () loc(#[[LOC]])
+"foo.op"() : () -> () loc(#loc)
diff --git a/mlir/test/IR/module-op.mlir b/mlir/test/IR/module-op.mlir
index 2d4ea839c137..2e5bd6f9685e 100644
--- a/mlir/test/IR/module-op.mlir
+++ b/mlir/test/IR/module-op.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt -allow-unregistered-dialect %s -split-input-file -mlir-print-debuginfo | FileCheck %s
+// RUN: mlir-opt -allow-unregistered-dialect %s -split-input-file -mlir-print-debuginfo -mlir-print-local-scope | FileCheck %s
// CHECK: module {
module {
diff --git a/mlir/test/IR/opaque_locations.mlir b/mlir/test/IR/opaque_locations.mlir
index b33ca4cfea2d..8d1dde8056ac 100644
--- a/mlir/test/IR/opaque_locations.mlir
+++ b/mlir/test/IR/opaque_locations.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt -allow-unregistered-dialect %s -test-opaque-loc -mlir-print-debuginfo | FileCheck %s
+// RUN: mlir-opt -allow-unregistered-dialect %s -test-opaque-loc -mlir-print-debuginfo -mlir-print-local-scope | FileCheck %s
// This test verifies that debug opaque locations can be printed.
#set0 = affine_set<(d0) : (1 == 0)>
diff --git a/mlir/test/IR/parser.mlir b/mlir/test/IR/parser.mlir
index aad4ba9e9c1e..ecb9ccc296ba 100644
--- a/mlir/test/IR/parser.mlir
+++ b/mlir/test/IR/parser.mlir
@@ -1002,9 +1002,6 @@ func @scoped_names() {
return
}
-// CHECK-LABEL: func @loc_attr(i1 {foo.loc_attr = loc(callsite("foo" at "mysource.cc":10:8))})
-func @loc_attr(i1 {foo.loc_attr = loc(callsite("foo" at "mysource.cc":10:8))})
-
// CHECK-LABEL: func @dialect_attribute_with_type
func @dialect_attribute_with_type() {
// CHECK-NEXT: foo = #foo.attr : i32
diff --git a/mlir/test/IR/wrapping_op.mlir b/mlir/test/IR/wrapping_op.mlir
index 661fb04518f4..54574f74fc35 100644
--- a/mlir/test/IR/wrapping_op.mlir
+++ b/mlir/test/IR/wrapping_op.mlir
@@ -1,5 +1,5 @@
// RUN: mlir-opt -allow-unregistered-dialect %s | FileCheck %s
-// RUN: mlir-opt -allow-unregistered-dialect -mlir-print-op-generic -mlir-print-debuginfo %s | FileCheck %s --check-prefix=CHECK-GENERIC
+// RUN: mlir-opt -allow-unregistered-dialect -mlir-print-op-generic -mlir-print-debuginfo -mlir-print-local-scope %s | FileCheck %s --check-prefix=CHECK-GENERIC
// CHECK-LABEL: func @wrapping_op
// CHECK-GENERIC: "func"
diff --git a/mlir/test/Transforms/inlining.mlir b/mlir/test/Transforms/inlining.mlir
index 54bf6c67d927..e59d8094768b 100644
--- a/mlir/test/Transforms/inlining.mlir
+++ b/mlir/test/Transforms/inlining.mlir
@@ -1,5 +1,5 @@
// RUN: mlir-opt %s -inline="disable-simplify" | FileCheck %s
-// RUN: mlir-opt %s -inline="disable-simplify" -mlir-print-debuginfo | FileCheck %s --check-prefix INLINE-LOC
+// RUN: mlir-opt %s -inline="disable-simplify" -mlir-print-debuginfo -mlir-print-local-scope | FileCheck %s --check-prefix INLINE-LOC
// RUN: mlir-opt %s -inline | FileCheck %s --check-prefix INLINE_SIMPLIFY
// Inline a function that takes an argument.
diff --git a/mlir/test/Transforms/location-snapshot.mlir b/mlir/test/Transforms/location-snapshot.mlir
index 8638efd3ccca..eada69f84cbf 100644
--- a/mlir/test/Transforms/location-snapshot.mlir
+++ b/mlir/test/Transforms/location-snapshot.mlir
@@ -1,5 +1,5 @@
-// RUN: mlir-opt -allow-unregistered-dialect -snapshot-op-locations='filename=%/t' -mlir-print-debuginfo %s | FileCheck %s -DFILE=%/t
-// RUN: mlir-opt -allow-unregistered-dialect -snapshot-op-locations='filename=%/t tag='tagged'' -mlir-print-debuginfo %s | FileCheck %s --check-prefix=TAG -DFILE=%/t
+// RUN: mlir-opt -allow-unregistered-dialect -snapshot-op-locations='filename=%/t' -mlir-print-local-scope -mlir-print-debuginfo %s | FileCheck %s -DFILE=%/t
+// RUN: mlir-opt -allow-unregistered-dialect -snapshot-op-locations='filename=%/t tag='tagged'' -mlir-print-local-scope -mlir-print-debuginfo %s | FileCheck %s --check-prefix=TAG -DFILE=%/t
// CHECK: func @function(
// CHECK-NEXT: loc("[[FILE]]":{{[0-9]+}}:{{[0-9]+}})
diff --git a/mlir/test/Transforms/strip-debuginfo.mlir b/mlir/test/Transforms/strip-debuginfo.mlir
index f1bf62e1d172..8cd915e775a0 100644
--- a/mlir/test/Transforms/strip-debuginfo.mlir
+++ b/mlir/test/Transforms/strip-debuginfo.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt -allow-unregistered-dialect %s -mlir-print-debuginfo -strip-debuginfo | FileCheck %s
+// RUN: mlir-opt -allow-unregistered-dialect %s -mlir-print-debuginfo -mlir-print-local-scope -strip-debuginfo | FileCheck %s
// This test verifies that debug locations are stripped.
#set0 = affine_set<(d0) : (1 == 0)>
diff --git a/mlir/test/mlir-tblgen/pattern.mlir b/mlir/test/mlir-tblgen/pattern.mlir
index a1ae18c05b3d..5496209d3886 100644
--- a/mlir/test/mlir-tblgen/pattern.mlir
+++ b/mlir/test/mlir-tblgen/pattern.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt -test-patterns -mlir-print-debuginfo %s | FileCheck %s
+// RUN: mlir-opt -test-patterns -mlir-print-debuginfo -mlir-print-local-scope %s | FileCheck %s
// CHECK-LABEL: verifyFusedLocs
func @verifyFusedLocs(%arg0 : i32) -> i32 {
More information about the Mlir-commits
mailing list