[Mlir-commits] [mlir] [mlir] [transform dialect] NamedSequenceOp build: honor arg_attrs when building (PR #168101)

[Jacques Pienaar]

================
@@ -0,0 +1,45 @@
+#include "mlir/Dialect/Transform/IR/TransformDialect.h"
+#include "mlir/Dialect/Transform/IR/TransformOps.h"
+#include "mlir/IR/Builders.h"
+#include "mlir/IR/BuiltinAttributes.h"
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/MLIRContext.h"
+#include "gtest/gtest.h"
+
+using namespace mlir;
+using namespace mlir::transform;
+
+TEST(NamedSequenceOpTest, ArgAttrsAreHonoredByBuilder) {
+  MLIRContext ctx;
+  ctx.loadDialect<TransformDialect>();
+
+  OpBuilder builder(&ctx);
+  auto module = ModuleOp::create(UnknownLoc::get(&ctx));
+  builder.setInsertionPointToEnd(module.getBody());
+
+  Location loc = UnknownLoc::get(&ctx);
+
+  static constexpr StringLiteral kMainSequenceName = "__transform_main";
+
+  NamedSequenceOp seqOp = builder.create<NamedSequenceOp>(
+      loc,
+      /*sym_name=*/kMainSequenceName,
+      /*rootType=*/builder.getType<AnyOpType>(),
+      /*resultType=*/TypeRange{},
+      [](OpBuilder &b, Location nested, Value rootH) {
+        b.create<YieldOp>(nested, ValueRange());
+      },
+      /*args=*/ArrayRef<NamedAttribute>{},
+      /*attrArgs=*/
+      ArrayRef<DictionaryAttr>{
+          builder.getDictionaryAttr(ArrayRef<NamedAttribute>{
+              builder.getNamedAttr(TransformDialect::kArgConsumedAttrName,
+                                   builder.getUnitAttr())})});
+
+  // 检查 body argument 上有没有 transform.consumed
+  Block &body = seqOp.getBody().front();
+  ASSERT_EQ(body.getNumArguments(), 1u);
+
+  StringAttr arg0Name = seqOp.getArgAttrsAttrName();
+  EXPECT_TRUE(arg0Name);
+}
----------------
jpienaar wrote:

Yes I was thinking something similar to Mehdi here. Well in order 
1. Ideally builders (not automatically generated) are tested via existing passes upstream and there is use of them. No additional worked needed and builder obviously useful.
2. Builder file say per dialect.

The pros and cons of builder unit test vs pass for me is, that unit test becomes its own binary (so extra linkage cost) but it doesn't bloat mlir-opt. Now, one could argue that mlir-opt is already a testing tool and so that doesn't actually matter. In which case Alex's suggestion is better.

I could see us doing a C++ unit test that looks like the Python lit tests (compile, run and check with lit). I am spoilt as I have a parallel build, but if we restrict it per dialect and only for those not covered in another way already, it's probably small set and not high cost while being very directed.

https://github.com/llvm/llvm-project/pull/168101