[Mlir-commits] [mlir] acab6a7 - Revert "Add a `skipRegion()` feature to the OpPrintingFlags for MLIR ASM printer"
Mehdi Amini
llvmlistbot at llvm.org
Mon Mar 13 09:50:07 PDT 2023
Author: Mehdi Amini
Date: 2023-03-13T17:49:47+01:00
New Revision: acab6a70fb40cc7d363bee047ef0a250416b77ad
URL: https://github.com/llvm/llvm-project/commit/acab6a70fb40cc7d363bee047ef0a250416b77ad
DIFF: https://github.com/llvm/llvm-project/commit/acab6a70fb40cc7d363bee047ef0a250416b77ad.diff
LOG: Revert "Add a `skipRegion()` feature to the OpPrintingFlags for MLIR ASM printer"
This reverts commit 0fe16607a523af3d8978ad636134e4d3034e365c which wasn't ready
to land.
Added:
Modified:
mlir/include/mlir/IR/OperationSupport.h
mlir/lib/IR/AsmPrinter.cpp
mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp
mlir/test/mlir-lsp-server/hover.test
Removed:
################################################################################
diff --git a/mlir/include/mlir/IR/OperationSupport.h b/mlir/include/mlir/IR/OperationSupport.h
index ba8f0a8bd79de..ebeb0a96523bd 100644
--- a/mlir/include/mlir/IR/OperationSupport.h
+++ b/mlir/include/mlir/IR/OperationSupport.h
@@ -834,9 +834,6 @@ class OpPrintingFlags {
/// Always print operations in the generic form.
OpPrintingFlags &printGenericOpForm();
- /// Skip printing regions.
- OpPrintingFlags &skipRegions();
-
/// Do not verify the operation when using custom operation printers.
OpPrintingFlags &assumeVerified();
@@ -864,9 +861,6 @@ class OpPrintingFlags {
/// Return if operations should be printed in the generic form.
bool shouldPrintGenericOpForm() const;
- /// Return if regions should be skipped.
- bool shouldSkipRegions() const;
-
/// Return if operation verification should be skipped.
bool shouldAssumeVerified() const;
@@ -888,9 +882,6 @@ class OpPrintingFlags {
/// Print operations in the generic form.
bool printGenericOpFormFlag : 1;
- /// Always skip Regions.
- bool skipRegionsFlag : 1;
-
/// Skip operation verification.
bool assumeVerifiedFlag : 1;
diff --git a/mlir/lib/IR/AsmPrinter.cpp b/mlir/lib/IR/AsmPrinter.cpp
index f4d66541e9cf7..8c5bb3021577d 100644
--- a/mlir/lib/IR/AsmPrinter.cpp
+++ b/mlir/lib/IR/AsmPrinter.cpp
@@ -183,9 +183,8 @@ void mlir::registerAsmPrinterCLOptions() {
/// Initialize the printing flags with default supplied by the cl::opts above.
OpPrintingFlags::OpPrintingFlags()
: printDebugInfoFlag(false), printDebugInfoPrettyFormFlag(false),
- printGenericOpFormFlag(false), skipRegionsFlag(false),
- assumeVerifiedFlag(false), printLocalScope(false),
- printValueUsersFlag(false) {
+ printGenericOpFormFlag(false), assumeVerifiedFlag(false),
+ printLocalScope(false), printValueUsersFlag(false) {
// Initialize based upon command line options, if they are available.
if (!clOptions.isConstructed())
return;
@@ -224,12 +223,6 @@ OpPrintingFlags &OpPrintingFlags::printGenericOpForm() {
return *this;
}
-/// Always skip Regions.
-OpPrintingFlags &OpPrintingFlags::skipRegions() {
- skipRegionsFlag = true;
- return *this;
-}
-
/// Do not verify the operation when using custom operation printers.
OpPrintingFlags &OpPrintingFlags::assumeVerified() {
assumeVerifiedFlag = true;
@@ -277,9 +270,6 @@ bool OpPrintingFlags::shouldPrintGenericOpForm() const {
return printGenericOpFormFlag;
}
-/// Return if Region should be skipped.
-bool OpPrintingFlags::shouldSkipRegions() const { return skipRegionsFlag; }
-
/// Return if operation verification should be skipped.
bool OpPrintingFlags::shouldAssumeVerified() const {
return assumeVerifiedFlag;
@@ -624,11 +614,9 @@ class DummyAliasOperationPrinter : private OpAsmPrinter {
/// Print the given operation in the generic form.
void printGenericOp(Operation *op, bool printOpName = true) override {
// Consider nested operations for aliases.
- if (!printerFlags.shouldSkipRegions()) {
- for (Region ®ion : op->getRegions())
- printRegion(region, /*printEntryBlockArgs=*/true,
- /*printBlockTerminators=*/true);
- }
+ for (Region ®ion : op->getRegions())
+ printRegion(region, /*printEntryBlockArgs=*/true,
+ /*printBlockTerminators=*/true);
// Visit all the types used in the operation.
for (Type type : op->getOperandTypes())
@@ -677,10 +665,6 @@ class DummyAliasOperationPrinter : private OpAsmPrinter {
bool printEmptyBlock = false) override {
if (region.empty())
return;
- if (printerFlags.shouldSkipRegions()) {
- os << "{/*skip region*/}";
- return;
- }
auto *entryBlock = ®ion.front();
print(entryBlock, printEntryBlockArgs, printBlockTerminators);
@@ -3357,14 +3341,10 @@ void OperationPrinter::printGenericOp(Operation *op, bool printOpName) {
// Print regions.
if (op->getNumRegions() != 0) {
os << " (";
- if (!printerFlags.shouldSkipRegions()) {
- interleaveComma(op->getRegions(), [&](Region ®ion) {
- printRegion(region, /*printEntryBlockArgs=*/true,
- /*printBlockTerminators=*/true, /*printEmptyBlock=*/true);
- });
- } else {
- os << "/*skip " << op->getNumRegions() << " regions*/";
- }
+ interleaveComma(op->getRegions(), [&](Region ®ion) {
+ printRegion(region, /*printEntryBlockArgs=*/true,
+ /*printBlockTerminators=*/true, /*printEmptyBlock=*/true);
+ });
os << ')';
}
@@ -3483,10 +3463,6 @@ void OperationPrinter::printSuccessorAndUseList(Block *successor,
void OperationPrinter::printRegion(Region ®ion, bool printEntryBlockArgs,
bool printBlockTerminators,
bool printEmptyBlock) {
- if (printerFlags.shouldSkipRegions()) {
- os << "{/*skip region*/}";
- return;
- }
os << "{" << newLine;
if (!region.empty()) {
auto restoreDefaultDialect =
diff --git a/mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp b/mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp
index 8597423946c79..397f2598ef73a 100644
--- a/mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp
+++ b/mlir/lib/Tools/mlir-lsp-server/MLIRServer.cpp
@@ -516,12 +516,23 @@ std::optional<lsp::Hover> MLIRDocument::buildHoverForOperation(
os << "Generic Form:\n\n```mlir\n";
- op.op->print(os, OpPrintingFlags()
- .printGenericOpForm()
- .elideLargeElementsAttrs()
- .skipRegions());
+ // Temporary drop the regions of this operation so that they don't get
+ // printed in the output. This helps keeps the size of the output hover
+ // small.
+ SmallVector<std::unique_ptr<Region>> regions;
+ for (Region ®ion : op.op->getRegions()) {
+ regions.emplace_back(std::make_unique<Region>());
+ regions.back()->takeBody(region);
+ }
+
+ op.op->print(
+ os, OpPrintingFlags().printGenericOpForm().elideLargeElementsAttrs());
os << "\n```\n";
+ // Move the regions back to the current operation.
+ for (Region ®ion : op.op->getRegions())
+ region.takeBody(*regions.back());
+
return hover;
}
diff --git a/mlir/test/mlir-lsp-server/hover.test b/mlir/test/mlir-lsp-server/hover.test
index 4fb9856442324..df0fe205c24a2 100644
--- a/mlir/test/mlir-lsp-server/hover.test
+++ b/mlir/test/mlir-lsp-server/hover.test
@@ -114,7 +114,7 @@
// CHECK-NEXT: "result": {
// CHECK-NEXT: "contents": {
// CHECK-NEXT: "kind": "markdown",
-// CHECK-NEXT: "value": "\"func.func\" : public @foo\n\nGeneric Form:\n\n```mlir\n\"func.func\"() (/*skip 1 regions*/) {function_type = (i1) -> (), sym_name = \"foo\"} : () -> ()\n```\n"
+// CHECK-NEXT: "value": "\"func.func\" : public @foo\n\nGeneric Form:\n\n```mlir\n\"func.func\"() ({\n}) {function_type = (i1) -> (), sym_name = \"foo\"} : () -> ()\n```\n"
// CHECK-NEXT: },
// CHECK-NEXT: "range": {
// CHECK-NEXT: "end": {
@@ -138,7 +138,7 @@
// CHECK-NEXT: "result": {
// CHECK-NEXT: "contents": {
// CHECK-NEXT: "kind": "markdown",
-// CHECK-NEXT: "value": "\"func.func\" : public @foo\n\nGeneric Form:\n\n```mlir\n\"func.func\"() (/*skip 1 regions*/) {function_type = (i1) -> (), sym_name = \"foo\"} : () -> ()\n```\n"
+// CHECK-NEXT: "value": "\"func.func\" : public @foo\n\nGeneric Form:\n\n```mlir\n\"func.func\"() ({\n}) {function_type = (i1) -> (), sym_name = \"foo\"} : () -> ()\n```\n"
// CHECK-NEXT: },
// CHECK-NEXT: "range": {
// CHECK-NEXT: "end": {
More information about the Mlir-commits
mailing list