This is an archive of the discontinued LLVM Phabricator instance.

[mlir][python] Implement more SymbolTable methods.
ClosedPublic

Authored by stellaraccident on Nov 28 2021, 8:34 PM.

Details

Summary
  • set_symbol_name, get_symbol_name, set_visibility, get_visibility, replace_all_symbol_uses, walk_symbol_tables
  • In integrations I've been doing, I've been reaching for all of these to do both general IR manipulation and module merging.
  • I don't love the replace_all_symbol_uses underlying APIs since they necessitate SYMBOL_COUNT walks and have various sharp edges. I'm hoping that whatever emerges eventually for this can still retain this simple API as a one-shot.

Diff Detail

Event Timeline

stellaraccident requested review of this revision.Nov 28 2021, 8:34 PM
ftynse accepted this revision.Nov 29 2021, 1:04 AM
ftynse added inline comments.
mlir/include/mlir-c/IR.h
797

Nit: this is already mentioned above.

809

Other similar callbacks, in particular in IR.h, take userData as the last argument. Let's do the same here for API consistency.

mlir/lib/Bindings/Python/IRCore.cpp
1599

Maybe this should return the attribute. I'm okay with both as long as it's the same for the entire SymbolTable API.

1601

Nit: MLIR code tends to expand auto whenever possible, e.g., here it's not clear if we are getting a PyOperation& or an Operation&.

1701–1703

Nit: throw std::runtime_error should work here.

1703

I don't think operator , works for Twine, did this mean + ?

This revision is now accepted and ready to land.Nov 29 2021, 1:04 AM
stellaraccident marked 5 inline comments as done.Nov 29 2021, 8:11 PM
stellaraccident added inline comments.
mlir/lib/Bindings/Python/IRCore.cpp
1703

Thanks for the catch.

Comments and rebase.

Add pyi bindings.

This revision was landed with ongoing or failed builds.Nov 29 2021, 8:32 PM
This revision was automatically updated to reflect the committed changes.