[Mlir-commits] [mlir] f54dc7b - [mlir][ODS] Omit printing default-valued attributes in oilists (#68880)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Fri Oct 13 02:22:11 PDT 2023
Author: Benjamin Maxwell
Date: 2023-10-13T10:22:05+01:00
New Revision: f54dc7b3936f1bd751db710cfc2fec1652159a3f
URL: https://github.com/llvm/llvm-project/commit/f54dc7b3936f1bd751db710cfc2fec1652159a3f
DIFF: https://github.com/llvm/llvm-project/commit/f54dc7b3936f1bd751db710cfc2fec1652159a3f.diff
LOG: [mlir][ODS] Omit printing default-valued attributes in oilists (#68880)
This makes these match the behaviour of optional attributes (which are
omitted when they are their default value of none). This allows for
concise assembly formats without a custom printer.
An extra print of " " is also removed, this does change any existing
uses of oilists, but if the parameter before the oilist is optional,
that would previously add an extra space.
This #68694 + some fixes for the MLIR Python tests, unfortunately GitHub
does not allow re-opening PRs :confused:
Added:
Modified:
flang/test/Lower/OpenMP/FIR/atomic-read.f90
flang/test/Lower/OpenMP/FIR/critical.f90
flang/test/Lower/OpenMP/critical.f90
mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
mlir/test/Dialect/OpenMP/ops.mlir
mlir/test/python/dialects/transform_structured_ext.py
mlir/test/python/dialects/transform_vector_ext.py
mlir/tools/mlir-tblgen/OpFormatGen.cpp
Removed:
################################################################################
diff --git a/flang/test/Lower/OpenMP/FIR/atomic-read.f90 b/flang/test/Lower/OpenMP/FIR/atomic-read.f90
index ff2b651953f2abc..0079b347fac8de6 100644
--- a/flang/test/Lower/OpenMP/FIR/atomic-read.f90
+++ b/flang/test/Lower/OpenMP/FIR/atomic-read.f90
@@ -14,7 +14,7 @@
!CHECK: %[[VAR_X:.*]] = fir.alloca i32 {bindc_name = "x", uniq_name = "_QFEx"}
!CHECK: %[[VAR_Y:.*]] = fir.alloca i32 {bindc_name = "y", uniq_name = "_QFEy"}
!CHECK: omp.atomic.read %[[VAR_X]] = %[[VAR_Y]] memory_order(acquire) hint(uncontended) : !fir.ref<i32>, i32
-!CHECK: omp.atomic.read %[[VAR_A]] = %[[VAR_B]] memory_order(relaxed) hint(none) : !fir.ref<!fir.char<1>>, !fir.char<1>
+!CHECK: omp.atomic.read %[[VAR_A]] = %[[VAR_B]] memory_order(relaxed) : !fir.ref<!fir.char<1>>, !fir.char<1>
!CHECK: omp.atomic.read %[[VAR_C]] = %[[VAR_D]] memory_order(seq_cst) hint(contended) : !fir.ref<!fir.logical<4>>, !fir.logical<4>
!CHECK: omp.atomic.read %[[VAR_E]] = %[[VAR_F]] hint(speculative) : !fir.ref<!fir.char<1,8>>, !fir.char<1,8>
!CHECK: omp.atomic.read %[[VAR_G]] = %[[VAR_H]] hint(nonspeculative) : !fir.ref<f32>, f32
diff --git a/flang/test/Lower/OpenMP/FIR/critical.f90 b/flang/test/Lower/OpenMP/FIR/critical.f90
index c6ac818fe21aa6e..b86729f8a98e370 100644
--- a/flang/test/Lower/OpenMP/FIR/critical.f90
+++ b/flang/test/Lower/OpenMP/FIR/critical.f90
@@ -2,7 +2,7 @@
!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | fir-opt --fir-to-llvm-ir | FileCheck %s --check-prefix="OMPDialect"
!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | fir-opt --fir-to-llvm-ir | tco | FileCheck %s --check-prefix="LLVMIR"
-!OMPDialect: omp.critical.declare @help2 hint(none)
+!OMPDialect: omp.critical.declare @help2
!OMPDialect: omp.critical.declare @help1 hint(contended)
subroutine omp_critical()
diff --git a/flang/test/Lower/OpenMP/critical.f90 b/flang/test/Lower/OpenMP/critical.f90
index 9fbd172df96421c..5a4d2e4815df49e 100644
--- a/flang/test/Lower/OpenMP/critical.f90
+++ b/flang/test/Lower/OpenMP/critical.f90
@@ -1,6 +1,6 @@
!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
-!CHECK: omp.critical.declare @help2 hint(none)
+!CHECK: omp.critical.declare @help2
!CHECK: omp.critical.declare @help1 hint(contended)
subroutine omp_critical()
diff --git a/mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir b/mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
index 1df27dd9957e594..881d738b413ef15 100644
--- a/mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
+++ b/mlir/test/Conversion/OpenMPToLLVM/convert-to-llvmir.mlir
@@ -90,7 +90,7 @@ func.func @wsloop(%arg0: index, %arg1: index, %arg2: index, %arg3: index, %arg4:
// CHECK-LABEL: @atomic_write
// CHECK: (%[[ARG0:.*]]: !llvm.ptr<i32>)
// CHECK: %[[VAL0:.*]] = llvm.mlir.constant(1 : i32) : i32
-// CHECK: omp.atomic.write %[[ARG0]] = %[[VAL0]] hint(none) memory_order(relaxed) : !llvm.ptr<i32>, i32
+// CHECK: omp.atomic.write %[[ARG0]] = %[[VAL0]] memory_order(relaxed) : !llvm.ptr<i32>, i32
func.func @atomic_write(%a: !llvm.ptr<i32>) -> () {
%1 = arith.constant 1 : i32
omp.atomic.write %a = %1 hint(none) memory_order(relaxed) : !llvm.ptr<i32>, i32
@@ -474,4 +474,4 @@ llvm.func @_QPtarget_map_with_bounds(%arg0: !llvm.ptr<i32>, %arg1: !llvm.ptr<arr
omp.terminator
}
llvm.return
-}
\ No newline at end of file
+}
diff --git a/mlir/test/Dialect/OpenMP/ops.mlir b/mlir/test/Dialect/OpenMP/ops.mlir
index 13cbea6c9923c22..27c31824c0506df 100644
--- a/mlir/test/Dialect/OpenMP/ops.mlir
+++ b/mlir/test/Dialect/OpenMP/ops.mlir
@@ -831,7 +831,7 @@ omp.critical.declare @mutex6 hint(contended, nonspeculative)
omp.critical.declare @mutex7 hint(uncontended, speculative)
// CHECK: omp.critical.declare @mutex8 hint(contended, speculative)
omp.critical.declare @mutex8 hint(contended, speculative)
-// CHECK: omp.critical.declare @mutex9 hint(none)
+// CHECK: omp.critical.declare @mutex9
omp.critical.declare @mutex9 hint(none)
// CHECK: omp.critical.declare @mutex10
omp.critical.declare @mutex10
@@ -909,7 +909,7 @@ func.func @omp_atomic_read(%v: memref<i32>, %x: memref<i32>) {
omp.atomic.read %v = %x hint(nonspeculative, contended) : memref<i32>, i32
// CHECK: omp.atomic.read %[[v]] = %[[x]] memory_order(seq_cst) hint(contended, speculative) : memref<i32>, i32
omp.atomic.read %v = %x hint(speculative, contended) memory_order(seq_cst) : memref<i32>, i32
- // CHECK: omp.atomic.read %[[v]] = %[[x]] memory_order(seq_cst) hint(none) : memref<i32>, i32
+ // CHECK: omp.atomic.read %[[v]] = %[[x]] memory_order(seq_cst) : memref<i32>, i32
omp.atomic.read %v = %x hint(none) memory_order(seq_cst) : memref<i32>, i32
return
}
@@ -927,7 +927,7 @@ func.func @omp_atomic_write(%addr : memref<i32>, %val : i32) {
omp.atomic.write %addr = %val memory_order(relaxed) : memref<i32>, i32
// CHECK: omp.atomic.write %[[ADDR]] = %[[VAL]] hint(uncontended, speculative) : memref<i32>, i32
omp.atomic.write %addr = %val hint(speculative, uncontended) : memref<i32>, i32
- // CHECK: omp.atomic.write %[[ADDR]] = %[[VAL]] hint(none) : memref<i32>, i32
+ // CHECK: omp.atomic.write %[[ADDR]] = %[[VAL]] : memref<i32>, i32
omp.atomic.write %addr = %val hint(none) : memref<i32>, i32
return
}
@@ -1004,7 +1004,7 @@ func.func @omp_atomic_update(%x : memref<i32>, %expr : i32, %xBool : memref<i1>,
omp.yield(%const:i32)
}
- // CHECK: omp.atomic.update hint(none) %[[X]] : memref<i32>
+ // CHECK: omp.atomic.update %[[X]] : memref<i32>
// CHECK-NEXT: (%[[XVAL:.*]]: i32):
// CHECK-NEXT: %[[NEWVAL:.*]] = llvm.add %[[XVAL]], %[[EXPR]] : i32
// CHECK-NEXT: omp.yield(%[[NEWVAL]] : i32)
@@ -1181,7 +1181,7 @@ func.func @omp_atomic_capture(%v: memref<i32>, %x: memref<i32>, %expr: i32) {
omp.atomic.write %x = %expr : memref<i32>, i32
}
- // CHECK: omp.atomic.capture hint(none) {
+ // CHECK: omp.atomic.capture {
// CHECK-NEXT: omp.atomic.update %[[x]] : memref<i32>
// CHECK-NEXT: (%[[xval:.*]]: i32):
// CHECK-NEXT: %[[newval:.*]] = llvm.add %[[xval]], %[[expr]] : i32
diff --git a/mlir/test/python/dialects/transform_structured_ext.py b/mlir/test/python/dialects/transform_structured_ext.py
index 0f89d4137455a41..c9b7802e1cc4532 100644
--- a/mlir/test/python/dialects/transform_structured_ext.py
+++ b/mlir/test/python/dialects/transform_structured_ext.py
@@ -439,7 +439,7 @@ def testTileToForallCompact(target):
structured.TileUsingForallOp(matmul, num_threads=[2, 3, 4])
# CHECK-LABEL: TEST: testTileToForallCompact
# CHECK: = transform.structured.tile_using_forall
- # CHECK-SAME: num_threads [2, 3, 4] tile_sizes []
+ # CHECK-SAME: num_threads [2, 3, 4]
# CHECK-SAME: (!transform.op<"linalg.matmul">) -> (!transform.any_op, !transform.any_op)
@@ -454,7 +454,7 @@ def testTileToForallLoopsAndTileOpTypes(target):
)
# CHECK-LABEL: TEST: testTileToForallLoopsAndTileOpTypes
# CHECK: = transform.structured.tile_using_forall
- # CHECK-SAME: num_threads [2, 3, 4] tile_sizes []
+ # CHECK-SAME: num_threads [2, 3, 4]
# CHECK-SAME: (!transform.any_op) -> (!transform.op<"scf.forall">, !transform.op<"linalg.matmul">)
@@ -464,7 +464,7 @@ def testTileToForallTileSizes(target):
structured.TileUsingForallOp(target, tile_sizes=[2, 3, 4])
# CHECK-LABEL: TEST: testTileToForallTileSizes
# CHECK: = transform.structured.tile_using_forall
- # CHECK-SAME: num_threads [] tile_sizes [2, 3, 4]
+ # CHECK-SAME: tile_sizes [2, 3, 4]
@run
diff --git a/mlir/test/python/dialects/transform_vector_ext.py b/mlir/test/python/dialects/transform_vector_ext.py
index 1a0a9e1d6ecbde7..a51f2154d1f7d54 100644
--- a/mlir/test/python/dialects/transform_vector_ext.py
+++ b/mlir/test/python/dialects/transform_vector_ext.py
@@ -94,30 +94,24 @@ def enum_configurable_patterns():
)
# CHECK: transform.apply_patterns.vector.lower_transpose
- # CHECK-SAME: lowering_strategy = eltwise
- # CHECK-SAME: avx2_lowering_strategy = false
vector.ApplyLowerTransposePatternsOp()
# CHECK: transform.apply_patterns.vector.lower_transpose
- # CHECK-SAME: lowering_strategy = eltwise
- # CHECK-SAME: avx2_lowering_strategy = false
+ # This is the default strategy, not printed.
vector.ApplyLowerTransposePatternsOp(
lowering_strategy=vector.VectorTransposeLowering.EltWise
)
# CHECK: transform.apply_patterns.vector.lower_transpose
# CHECK-SAME: lowering_strategy = flat_transpose
- # CHECK-SAME: avx2_lowering_strategy = false
vector.ApplyLowerTransposePatternsOp(
lowering_strategy=vector.VectorTransposeLowering.Flat
)
# CHECK: transform.apply_patterns.vector.lower_transpose
# CHECK-SAME: lowering_strategy = shuffle_1d
- # CHECK-SAME: avx2_lowering_strategy = false
vector.ApplyLowerTransposePatternsOp(
lowering_strategy=vector.VectorTransposeLowering.Shuffle1D
)
# CHECK: transform.apply_patterns.vector.lower_transpose
# CHECK-SAME: lowering_strategy = shuffle_16x16
- # CHECK-SAME: avx2_lowering_strategy = false
vector.ApplyLowerTransposePatternsOp(
lowering_strategy=vector.VectorTransposeLowering.Shuffle16x16
)
diff --git a/mlir/tools/mlir-tblgen/OpFormatGen.cpp b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
index bdb97866a47fc9d..18ca34379a71a0e 100644
--- a/mlir/tools/mlir-tblgen/OpFormatGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpFormatGen.cpp
@@ -2009,6 +2009,16 @@ static void genEnumAttrPrinter(const NamedAttribute *var, const Operator &op,
" }\n";
}
+/// Generate a check that a DefaultValuedAttr has a value that is non-default.
+static void genNonDefaultValueCheck(MethodBody &body, const Operator &op,
+ AttributeVariable &attrElement) {
+ FmtContext fctx;
+ Attribute attr = attrElement.getVar()->attr;
+ fctx.withBuilder("::mlir::OpBuilder((*this)->getContext())");
+ body << " && " << op.getGetterName(attrElement.getVar()->name) << "Attr() != "
+ << tgfmt(attr.getConstBuilderTemplate(), &fctx, attr.getDefaultValue());
+}
+
/// Generate the check for the anchor of an optional group.
static void genOptionalGroupPrinterAnchor(FormatElement *anchor,
const Operator &op,
@@ -2042,12 +2052,7 @@ static void genOptionalGroupPrinterAnchor(FormatElement *anchor,
if (attr.hasDefaultValue()) {
// Consider a default-valued attribute as present if it's not the
// default value.
- FmtContext fctx;
- fctx.withBuilder("::mlir::OpBuilder((*this)->getContext())");
- body << " && " << op.getGetterName(element->getVar()->name)
- << "Attr() != "
- << tgfmt(attr.getConstBuilderTemplate(), &fctx,
- attr.getDefaultValue());
+ genNonDefaultValueCheck(body, op, *element);
return;
}
llvm_unreachable("attribute must be optional or default-valued");
@@ -2158,7 +2163,6 @@ void OperationFormat::genElementPrinter(FormatElement *element,
// Emit the OIList
if (auto *oilist = dyn_cast<OIListElement>(element)) {
- genLiteralPrinter(" ", body, shouldEmitSpace, lastWasPunctuation);
for (auto clause : oilist->getClauses()) {
LiteralElement *lelement = std::get<0>(clause);
ArrayRef<FormatElement *> pelement = std::get<1>(clause);
@@ -2170,8 +2174,14 @@ void OperationFormat::genElementPrinter(FormatElement *element,
for (VariableElement *var : vars) {
TypeSwitch<FormatElement *>(var)
.Case([&](AttributeVariable *attrEle) {
- body << " || " << op.getGetterName(attrEle->getVar()->name)
+ body << " || (" << op.getGetterName(attrEle->getVar()->name)
<< "Attr()";
+ Attribute attr = attrEle->getVar()->attr;
+ if (attr.hasDefaultValue()) {
+ // Don't print default-valued attributes.
+ genNonDefaultValueCheck(body, op, *attrEle);
+ }
+ body << ")";
})
.Case([&](OperandVariable *ele) {
if (ele->getVar()->isVariadic()) {
More information about the Mlir-commits
mailing list