This is an archive of the discontinued LLVM Phabricator instance.

[mlir] use absolute import path in python op bindings generator
AbandonedPublic

Authored by ftynse on Apr 20 2021, 8:01 AM.

Details

Summary

The op bindings generator currently emits Python code with relative
import ._ods_common that implicitly assumes the file is being emitted
in mlir/lib/Bindings/Python/mlir/dialects, which is where _ods_common.py
resides. This is not necessarily the case for out-of-tree dialects. Use
an absolute path instead. This is expected to work as long as `import
mlir` does.

Diff Detail

Event Timeline

ftynse created this revision.Apr 20 2021, 8:01 AM
ftynse requested review of this revision.Apr 20 2021, 8:01 AM
nicolasvasilache accepted this revision.Apr 20 2021, 8:05 AM
This revision is now accepted and ready to land.Apr 20 2021, 8:05 AM

Just a sec: we solved this on the circt side - need to look at how.

(We do have upcoming use cases where we are going to want subprojects to be able to have completely isolated, private APIs, and i am trying to keep things relocatable)

I'm other projects, we include an _ods_common.py trampoline in the project specific dialects directory: https://github.com/llvm/circt/blob/main/lib/Bindings/Python/circt/dialects/_ods_common.py

Not hard coding the top level package in tablegen enables relocatable/private instances of the API, which will be important in the future.

Having to copy trampoline code and list the imports expected by tablegenerated code in every project feels fragile. WDYT about adding a tablegen command-line flag that prefixes imports with the given string, empty by default?

Couldn't there be a template trampoline code file that folks could copy/symlink ?

Having to copy trampoline code and list the imports expected by tablegenerated code in every project feels fragile. WDYT about adding a tablegen command-line flag that prefixes imports with the given string, empty by default?

Six of one/half dozen of the other: C++ based tablegen modules has a fair amount of fragile "spell it this way or else" things that come along with it. I ultimately don't care outside of a weak preference to encode fewer load bearing special things in the build system. A command line flag as you say is fine with me.

ftynse abandoned this revision.May 17 2021, 7:30 AM