PSA: `OpTy::create` now with 100% more tab complete

MLIR
1

TL;DR: This PR [mlir][tblgen] add concrete create methods has landed. That means OpTy::create APIs are now available for creating ops.

They look almost exactly like the conventional/prior builder.create<OpTy>(...) APIs but instead pass the builder as the first arg: OpTy::create(builder, ...). E.g.,

static arith::ConstantIndexOp create(OpBuilder& builder, Location location, int64_t value);

The advantage is they will tab complete, showing all of the possible generated builder signatures:

image
image1826Ă—926 213 KB

Note, both OpBuilder and ImplicitLocBuilder are supported.

Regarding deprecation

Given the importance of this particular API, we are planning to deprecate the prior builder.create API slowly. So deprecation after the next release branches, and removal one or two releases after that (probably two).

For downstream users, the migration path is straightforward:

  1. A regex like (\w+)\.create<(.*?)>\( → $2::create($1,;
  2. Hand decls/defns only for ops not defined in tablegen (like arith::ConstantIntOp);
    • Conversely, if all of your ops are defined in tablegen, you don’t need to do anything.

I will be migrating upstream to these new APIs over the next couple of weeks (see this PR).

EDIT: if you have “dangling” builders, like this

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

then this PR will break you in the sense that you will no longer be able to link (because the generated create methods will reference the non-existent defns of those dangling decls). The solution is to remove the dangling builder decls from your tablegen.

17 Likes
2

FYI anyone trying find/regex these “old” creates can do something like

 find . -name '*.cpp' -exec sed -E -i  \
   's/(\w+)[[:space:]]*\.[[:space:]]*create<([^>]*)>\(/\2::create(\1, /g' {} +

(thanks @newling)

3 Likes
3

When the upstream migration is complete, it would be nice to somehow prevent new usages from being added. I don’t know if marking the old style deprecated is the way to go since it will be extremely spammy downstream…

4

I’m in the process of adding a clang-tidy check (still under review). It would enable flagging as well as fixing.

5 Likes
5

To me this seems like a very similar situation to what we had when we deprecated member cast functions (foo.dyn_cast<T>()) – I’d expect an eventual deprecation attribute placed on those builder methods and eventual removal a couple of releases down the road.

BTW, @makslevental since you are already halfway through the batched upgrades, do you want to add a soft deprecation notice to Deprecations & Current Refactoring - MLIR ?

2 Likes
6

sure I’ll send a PR.

7

llvm-use-new-mlir-op-builder clang-tidy check landed. (What I’ve done previously was to just set it cmake side, build and apply fixes).

6 Likes
8

Very cool, thanks for improving this!

1 Like
9

With [mlir][NFC] update `mlir/lib` create APIs (35/n) by makslevental · Pull Request #150708 · llvm/llvm-project · GitHub landed the upstream deprecation of OpBuilder::create in favor of the generated OpTy::create APIs is complete. Let me know if you have any trouble downstream.

Shoutout @kuhar for reviewing many of the PRs.

2 Likes